YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.43k stars 875 forks source link

for loop consumes extreme amount of memory #3447

Open kranerup opened 2 years ago

kranerup commented 2 years ago

Steps to reproduce the issue

loop_issue.txt

yosys> read_verilog -sv loop_issue.txt

Yosys version used: Yosys 0.9+4081 (git sha1 862e84eb, clang 10.0.0-4ubuntu1 -fPIC -Os)

Expected behavior

Yosys should be able to read the verilog file without consuming excessive amount of memory.

Actual behavior

Yosys uses a huge amount of memory for this loop. It runs out of memory on a server with 32 Gbyte RAM. If I change to loop to 20 iterations then it will suceed to read the file.

jix commented 2 years ago

Is there a reason you use regi instead of i for indexing into dut_odata_mask and dut_odata_x? This runs into a limitation of the verilog frontend and prevents it from detecting that the index is constant. Using dut_odata_mask[i] and dut_odata_x[i] instead works fine here, as it detects i as loop variable.

When the frontend does not detect that the index is constant, it has to handle it as a dynamic index. Since this is an always_comb block and not clocked, the dynamic indexing is converted into a 1440 wide case statement for each possible index. (In a synchronous context it is possible to use more efficient shifts instead). This happens for every loop iteration, so after unrolling, the frontend has to handle ~2 million cases in a single block and runs out of memory.

Currently I don't know what would be required to make the frontend detect that dut_odata_mask[regi] and dut_odata_x[regi] use constant indices in this context.

kranerup commented 2 years ago

Thanks for the quick response. We have recently changed this code to use regi instead of "i" as index because using "i" causes SpyGlass warnings. Unfortunately we have a lot of code with for loops like this.

Best regards, Kenny

On Wed, 17 Aug 2022 at 15:54, Jannis Harder @.***> wrote:

Is there a reason you use regi instead of i for indexing into dut_odata_mask and dut_odata_x? This runs into a limitation of the verilog frontend and prevents it from detecting that the index is constant. Using dut_odata_mask[i] and dut_odata_x[i] instead works fine here, as it detects i as loop variable.

When the frontend does not detect that the index is constant, it has to handle it as a dynamic index. Since this is an always_comb block and not clocked, the dynamic indexing is converted into a 1440 wide case statement for each possible index. (In a synchronous context it is possible to use more efficient shifts instead). This happens for every loop iteration, so after unrolling, the frontend has to handle ~2 million cases in a single block and runs out of memory.

Currently I don't know what would be required to make the frontend detect that dut_odata_mask[regi] and dut_odata_x[regi] use constant indices in this context.

— Reply to this email directly, view it on GitHub https://github.com/YosysHQ/yosys/issues/3447#issuecomment-1218042589, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4PL3455NDCF5QI7MPD2PLVZTVIRANCNFSM56ZN4KTQ . You are receiving this because you authored the thread.Message ID: @.***>

jix commented 2 years ago

I usually don't bring this up for issue reports on GitHub, but since you mention that you are already using commercial tools, would the version of Yosys included with Tabby CAD be an option for you? It comes with the Verific SystemVerilog frontend that has very high compatibility with other industry standard tools and handles loops like this without any problems. If you want to give it a try, you can request an evaluation license.