verilog-to-routing / vtr-verilog-to-routing

Verilog to Routing -- Open Source CAD Flow for FPGA Research
https://verilogtorouting.org
Other
1.01k stars 388 forks source link

Compatibility issues regarding the 27x27 multiplier architecture of stratix10_arch and the Koios benchmark #2292

Closed jdzhu19 closed 1 year ago

jdzhu19 commented 1 year ago

Expected Behaviour

The synthesis should go smoothly.

Current Behaviour

I found that stratix10_arch and COFFE_22nm/k6FracN10LB_mem20K_complexDSP_customSB_22nm.xml with a similar 27x27 multiplier architecture seem to report the following error at the vpr stage : Error 1. Type: Blif file File: test.pre-vpr.blif Line: 1455 Message: Port 'a[27]' on architecture model 'multiply' exceeds port width (27 bits) This problem does not occur in the 36x36 case, I would like to check with you if this is normal. BTW, the synthesizer I use is yosys.

Steps to Reproduce

Just use yosys to do the synthesis.

Context

Some time ago I wanted to test stratix10_arch on Koios to compare with my own arch.

Your Environment

jdzhu19 commented 1 year ago

@alirezazd Could you please help me take a look at this issue?

alirezazd commented 1 year ago

@jdzhu19 Can you please provide the exact steps to reproduce the error? What version of Yosys are you using? What are the commands that you use to synthesize your design? Can you provide the design or a problematic design file?

jdzhu19 commented 1 year ago

@jdzhu19 Can you please provide the exact steps to reproduce the error? What version of Yosys are you using? What are the commands that you use to synthesize your design? Can you provide the design or a problematic design file?

Yosys 0.27+12 (git sha1 ceef00c35, clang 10.0.0-4ubuntu1 -fPIC -Os) in commit f669015. I use script_params_common=-track_memory_usage -crit_path_router_iterations 100 -start yosys in the task. No matter what the design is, the above error will be reported. @vaughnbetz thought that it maybe a problem in describing the max port width to yosys.

alirezazd commented 1 year ago

@jdzhu19 can you please give the exact task path?

jdzhu19 commented 1 year ago

@jdzhu19 can you please give the exact task path?

This is a task I built myself. The preset task in the VTR does not seem to contain the architecture of stratix10_arch or k6FracN10LB_mem20K_complexDSP_customSB_22nm.xml? I'm not sure, but I think that you can create a new task to try to restore this error, whether you use VTR benchmark or Koios. Thanks for your time.

alirezazd commented 1 year ago

@jdzhu19 Thank you, I would like to suggest that you read our Contribution Guideline. As a team with limited resources, it is crucial that we receive detailed information regarding the issue you have encountered in order to address it effectively. We kindly request that you provide us with the exact steps to reproduce the issue, as this information can often lead to resolved issues automatically. Your cooperation in this matter is greatly appreciated.

Regarding your custom task, please keep in mind that there are many variables that can affect the synthesis process, and we must take all of them into account to provide an accurate solution. Additionally, it is important to note that the flow you are currently using is deprecated, and the issue you encountered may be related to the large multiplier width, which is not supported in the deprecated flow. Therefore, we suggest using the updated flow version with the Parmys plugin to avoid this issue.

If you require further assistance, my colleague @navidjafarof will be available to assist you with your issue while I attend to other important tasks.

Thank you for your understanding and cooperation.

jdzhu19 commented 1 year ago

Sorry, I will provide more detailed steps in this answer. Detailed steps to re-produce the bug: (1) Build f669015 (2) Run the task with following content:

# Path to directory of circuits to use
circuits_dir=benchmarks/verilog/koios

# Path to directory of architectures to use
archs_dir=arch/COFFE_22nm

# Add circuits to list to sweep 
circuit_list_add=test.v  

# Add architectures to list to sweep
arch_list_add=k6FracN10LB_mem20K_complexDSP_customSB_22nm.xml
arch_list_add=stratix10_arch.xml

# Parse info and how to parse
parse_file=vpr_standard.txt

# How to parse QoR info
qor_parse_file=qor_standard.txt

# Pass requirements. Allow you to compare an executed task to a golden reference result
pass_requirements_file=pass_requirements.txt

# Common parameters to be passed to all script invocations
script_params_common=-track_memory_usage -crit_path_router_iterations 100 -start yosys

Then the error that I mentioned will occur.

If you want to know more information, please continue to contact me, thank you very much for your time. By the way, I spent a lot of time trying many times the ODIN+Yosys or parmys versions, but for some reason, I can't make them successfully, I would be very grateful if you can help me solve this problem of single yosys as the synthesizer.

vaughnbetz commented 1 year ago

We recently updated the required cmake version to resolve an issue building parmys with older versions. You can try pulling the latest code to see if that resolves the build issue. If it doesn't, we'd need an exact sequence of steps and an error message to be able to make any progress.

alirezazd commented 1 year ago

@jdzhu19 I believe the issue stems from the multiplier width being fixed at 36. Only this specific width is inferred correctly, which leads to the failure for other widths. The problem is due to the usage of an outdated commit that has been deprecated. You can find the related code parts below:

https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/f669015f3c7adf4347b5cb6e7b4013ef0c9c31bb/vtr_flow/misc/yosyslib/yosys_models.v#L24 https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/f669015f3c7adf4347b5cb6e7b4013ef0c9c31bb/vtr_flow/misc/yosyslib/multiply.v

To resolve the issue, it is crucial that you update to the latest version of the codebase, which includes the Parmys plugin. This updated version offers better support, simplified maintenance, and increased flexibility for handling different multiplier widths.

Once you have switched to the new version, please make sure to fully compile and test your code. If you encounter any issues during this process, kindly follow the guidelines and create a new issue with the necessary information.

I urge you to update to the latest version and utilize the Parmys plugin for a smoother experience. If you need any help or have further questions, don't hesitate to reach out. In light of this, I would recommend closing this issue as it is not relevant to the current version of the tool.

jdzhu19 commented 1 year ago

We recently updated the required cmake version to resolve an issue building parmys with older versions. You can try pulling the latest code to see if that resolves the build issue. If it doesn't, we'd need an exact sequence of steps and an error message to be able to make any progress.

Many thanks, my cmake version is 3.19.7, I don't know if there are recent changes to the make files relevant to the cmake requirements. Until a few days ago, I still used the latest VTR version to build and failed. I will try the latest version today.

jdzhu19 commented 1 year ago

@jdzhu19 I believe the issue stems from the multiplier width being fixed at 36. Only this specific width is inferred correctly, which leads to the failure for other widths. The problem is due to the usage of an outdated commit that has been deprecated. You can find the related code parts below:

https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/f669015f3c7adf4347b5cb6e7b4013ef0c9c31bb/vtr_flow/misc/yosyslib/yosys_models.v#L24

https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/f669015f3c7adf4347b5cb6e7b4013ef0c9c31bb/vtr_flow/misc/yosyslib/multiply.v To resolve the issue, it is crucial that you update to the latest version of the codebase, which includes the Parmys plugin. This updated version offers better support, simplified maintenance, and increased flexibility for handling different multiplier widths.

Once you have switched to the new version, please make sure to fully compile and test your code. If you encounter any issues during this process, kindly follow the guidelines and create a new issue with the necessary information.

I urge you to update to the latest version and utilize the Parmys plugin for a smoother experience. If you need any help or have further questions, don't hesitate to reach out. In light of this, I would recommend closing this issue as it is not relevant to the current version of the tool.

Thank you, I will try the latest updated version as soon as possible today.

jdzhu19 commented 1 year ago

We recently updated the required cmake version to resolve an issue building parmys with older versions. You can try pulling the latest code to see if that resolves the build issue. If it doesn't, we'd need an exact sequence of steps and an error message to be able to make any progress.

I tried it, but unfortunately it is exactly the same as the error I mentioned in issue#2267, and in my many previous attempts I found that since commit 1599a5c, this error will appear. My system version is centOS7, my cmake version is 3.19.7, my gcc version is gcc version 12.2.0 (GCC), as answered in that issue, maybe it's my local problem, but I haven't found the source of the problem, maybe I will continue to try to solve it later.