Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add vtr-optimized, with PGO enabled. #60

Merged
merged 9 commits into from
Dec 16, 2020
Merged

Add vtr-optimized, with PGO enabled. #60

merged 9 commits into from
Dec 16, 2020

Conversation

HackerFoo
Copy link
Contributor

@HackerFoo HackerFoo commented Dec 14, 2020

This PR adds a version of VTR that is optimized using profile-guided optimization, which reduces runtime by ~8%.

To do this, it must download a sample architecture and circuit, and run all steps to gather profile data, and then recompile using the profile data collected.

This PR supersedes SymbiFlow/conda-packages#143

Signed-off-by: Dusty DeWeese <dustin.deweese@gmail.com>
Signed-off-by: Dusty DeWeese <dustin.deweese@gmail.com>
@HackerFoo HackerFoo changed the title Add symbiflow-vtr-optimized, with PGO enabled. Add vtr-optimized, with PGO enabled. Dec 14, 2020
Copy link
Contributor

@PiotrZierhoffer PiotrZierhoffer left a comment

Choose a reason for hiding this comment

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

I restarted the failing jobs (there was a problem with github connection), but from the packaging perspective - lgtm.

I am not sure if @litghost ACKs these changes as well or is there still something to be fixed?

@PiotrZierhoffer
Copy link
Contributor

If you let me know it's good to go I'll merge right away

Copy link
Contributor

@litghost litghost left a comment

Choose a reason for hiding this comment

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

LGTM, merge if green. Would also be good to get some concrete % improvement numbers.

Copy link
Contributor

@litghost litghost left a comment

Choose a reason for hiding this comment

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

Looks like an ibex workaround file was left in the build.sh?

@HackerFoo
Copy link
Contributor Author

Looks like an ibex workaround file was left in the build.sh?

I've removed it.

@litghost
Copy link
Contributor

Error https://travis-ci.com/github/litex-hub/litex-conda-eda/jobs/460723313#L1245:

  File "/home/travis/build/litex-hub/litex-conda-eda/workdir/conda-env/lib/python3.7/site-packages/conda/resolve.py", line 288, in verify_specs
    raise ResolvePackageNotFound(bad_deps)
.
  - python-constraint

The key here is that you want to do a pip install, not install a conda package. Example: https://github.com/SymbiFlow/symbiflow-arch-defs/blob/442941d382bdbe212c1a9b153e2c623aa2fd707d/environment.yml#L29
You can also include a requirements.txt, example: https://github.com/SymbiFlow/symbiflow-arch-defs/blob/7183cb5e59d50955686c12e4b800a55fafd1fae5/environment.yml#L24-L26

@HackerFoo
Copy link
Contributor Author

HackerFoo commented Dec 16, 2020

ValueError: Dictionaries are not supported as values in requirements sections. Note that pip requirements as used in conda-env environment.yml files are not supported by conda-build.

I guess I need to use the Conda version.

Signed-off-by: Dusty DeWeese <dustin.deweese@gmail.com>
Signed-off-by: Dusty DeWeese <dustin.deweese@gmail.com>
Signed-off-by: Dusty DeWeese <dustin.deweese@gmail.com>
Signed-off-by: Dusty DeWeese <dustin.deweese@gmail.com>
Signed-off-by: Dusty DeWeese <dustin.deweese@gmail.com>
Signed-off-by: Dusty DeWeese <dustin.deweese@gmail.com>
Signed-off-by: Dusty DeWeese <dustin.deweese@gmail.com>
@HackerFoo
Copy link
Contributor Author

LGTM, merge if green. Would also be good to get some concrete % improvement numbers.

For minilitex_ddr_arty:

Phase vtr vtr-optimized Improvement
fasm 32.2 28.0 13%
pack 99.7 97.2 2.5%
place 49.3 43.0 12%
route 108 84.9 22%
TOTAL 289 253 13%

@HackerFoo HackerFoo merged commit 498fefa into hdl:master Dec 16, 2020
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.

None yet

4 participants