-
Notifications
You must be signed in to change notification settings - Fork 396
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
Yosys42 #2646
Yosys42 #2646
Conversation
@vaughnbetz I'm testing the final commits, but the CI test seems to be stuck and never succeeds. Also, "pushing an empty commit" or "closing and reopening the PR" didn't work. I switched to a new branch(this branch), but the problem persists. |
Not sure what the problem is. Perhaps CI has gotten overloaded again? (if so, try later tonight and it should work). @AlexandreSinger : any ideas? |
Based on the fact that the tests are saying "Expected - Waitiing for status to be reported"; I think that the CI is refusing to run this PR since the title says "WIP". I removed WIP from the title, but it still seems to be stalled. I noticed that this branch has conflicts with |
I had changed the test.yml(disable system verilog test) but I was able to continue with .github/workflows/test.yml conflict warning successfully last time( all tests ran). but this time I am getting this issue. |
I am not sure then what could be the root cause; but that (along with the WIP status) were the only differences between this PR and other that are able to run the CI. Regardless the conflicts need to be resolved before the code can be merged. |
ba0d1e9
to
273d7b2
Compare
@amirarjmand93 It looks like resolving the conflicts seem to have launched the CI. The vtr_reg_nightly failures seems to be an infrastructure issue on VTR's end. The self-hosted CI runners are not responding. This is happening on other runs; I am looking into it. However, the "R: Basic ..." tests seem to be issue with your PR: |
f839aab
to
dad6580
Compare
all VTR and Parmys golden results are committed. |
Yes, the self hosted runners are down ... we'll discuss tomorrow but perhaps they have been deactivated in which case we'll have to come up with a new strategy. |
I cancelled the CI since there is a build issue that needs to be resolved. |
Thanks, @vaughnbetz and @AlexandreSinger. |
Nice detective work @amirarjmand93 and @AlexandreSinger ! |
@amirarjmand93 Your last commit disabled MCML and LU32 in vtr_reg_qor_chain, which was not the testcase which was timing out (which is why the CI still timed out). We need to disable these circuits in |
@vaughnbetz Nightly1 is causing problems again due to small fractional values. by local PC, all tests pass. Probably, we should reconsider the pass_requirement file(Run_time metrics): |
I'm fine with cutting the lower limit for min_chan_width route time to 0.05. I suggest going ahead and making that change. |
Finally done :) |
CI is green! Yaay! Thanks for driving this @amirarjmand93 ! @AlexandreSinger : I think you wanted to do a QoR run to check the parsing scripts & get a full comparison (VTR benchmarks and some of the Koios ones I think). If you can get that soon I can wait and then merge. I'd like to get this in fairly soon though ... |
@vaughnbetz Yes, that is correct! I will do that this evening! We should hopefully be able to land this by tomorrow morning. I'll add some commits if there is something that needs to be added. |
The changes made to Yosys added a few random lines which I removed. Added some comments explaining why certain tests were turned off. Upgraded libcatch to match master since the PR would have downgraded master by mistake.
@amirarjmand93 I made some small changes. Mainly just removing some extra lines, adding some comments on why the tests were turned off, and upgrading the libcatch module (since it was going to overwrite master if that was not upgraded properly). @vaughnbetz I did a QoR comparison for MCNC (to get some quick results) and the comparison looked mostly fine to me (no data was missing that the old parse_script was not also missing). I will run VTR tomorrow, since it will take a few hours to run, but I am confident that any problems can be fixed-up in another PR. Great job on this PR @amirarjmand93 ; thank you so much for the hard work! |
I dropped some comments for that but without much explanation. |
@vaughnbetz This LGTM. I'll run the VTR tests this morning for a QoR test to try and see if the scripts have any issues; but I think this can be merged in the meantime. |
@vaughnbetz The VTR results are complete. The compare QoR script worked exactly as expected. Here is the summary:
Here is the data per circuit:
I noticed that I think this PR is good to go. Here is the raw data in case you were curious: |
Excellent! Thanks @amirarjmand93 and @AlexandreSinger ! |
Description
This update brings Yosys to version 0.42, which includes several bug fixes, performance improvements, and new features that enhance the synthesis capabilities of our flow. Due to incompatibility with the latest Yosys, F4PGA plug-in related tests and tasks have been disabled in the flow until a new plugin is installed.
According to the QoR results, there is a significant improvement in time and area from synthesis to routing in most tests and architectures.
Related Issue
#- {test: "vtr_reg_system_verilog", cores: "16", options: "", cmake: "-DYOSYS_F4PGA_PLUGINS=ON",
- {test: "parmys_reg_strong", cores: "16", options: "", cmake: "-DYOSYS_F4PGA_PLUGINS=OFF",
Important Features Added to Yosys from Versions 32 to 42
General Enhancements
New Commands and Options
Optimization and Synthesis Enhancements
Advanced Features
Intel Support
Types of changes
Checklist: