Skip to content

Conversation

@danc86
Copy link
Contributor

@danc86 danc86 commented Sep 30, 2021

No description provided.

@danc86 danc86 requested a review from alanvgreen September 30, 2021 07:51
@danc86
Copy link
Contributor Author

danc86 commented Sep 30, 2021

FYI @kgugala this works nicely on HPS, and gives a reduction in resource usage. But I confirmed this only reduces model evaluation time by ~0.35%.

Copy link
Contributor

@kgugala kgugala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kgugala
Copy link
Contributor

kgugala commented Sep 30, 2021

FYI @kgugala this works nicely on HPS, and gives a reduction in resource usage. But I confirmed this only reduces model evaluation time by ~0.35%.

is this with picolibc and LTO?

@danc86
Copy link
Contributor Author

danc86 commented Sep 30, 2021

Yes, these are the numbers I have in my spreadsheet running on HPS proto2 board, evaluating HPS model.

commit 10400f2 (last week)
= 193473536 cycles

commit 740150c (current main)
= 176910336 cycles

this PR
= 176288768 cycles

It suggests picolibc and LTO give a good speed improvement, as you reported, but effectively no change from doubling the SPI flash clock. It doesn't seem right.

@kgugala
Copy link
Contributor

kgugala commented Sep 30, 2021

maybe to biggest bottleneck is not in flash now?

@enjoy-digital
Copy link

@danc86, @kgugala: I'd like to merge litex-hub/litespi#60 that simplifies DDR mode but need feedback (I was only able to test on Arty), could you do a test with it? Thanks.

@danc86
Copy link
Contributor Author

danc86 commented Oct 6, 2021

@danc86, @kgugala: I'd like to merge litex-hub/litespi#60 that simplifies DDR mode but need feedback (I was only able to test on Arty), could you do a test with it? Thanks.

I tried with that PR applied and it doesn't work (CPU never boots from SPI flash). I haven't looked closer at why not. I'll write a comment with more details on that Litespi PR.

@danc86
Copy link
Contributor Author

danc86 commented Oct 6, 2021

I want to do more investigation on this (including to understand why there is no effective difference in model evaluation time) but I think we should merge it now and continue from here.

@danc86 danc86 merged commit 4b52a01 into google:main Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants