YosysHQ / yosys

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

Error Arises During Synthesis due to Verilog Code Structure #4079

Open DeLoSoCode opened 7 months ago

DeLoSoCode commented 7 months ago

Version

yosys 0.35+56

On which OS did this happen?

Linux

Reproduction Steps

Hello, While engaging in the synthesis process with Yosys, I faced a challenge when read the design file (rtl.v). Executing the read_verilog rtl.v command led to the error message captured in the attached screenshot. This occurred during attempts to optimize and synthesize the design using Yosys tools. image Synthesis processes as follows: read_verilog rtl.v
synth write_verilog syn_yosys.v

The synthesis of the design file (rtl.v) is successful when employing Vivado synthesis tools. This success suggests that the issue is specific to Yosys.

Please find attached the code of RTL . Thank you in advance for your attention to this matter. I look forward to hearing from you regarding this issue. yosys_project_1215.zip

Expected Behavior

synthesis success

Actual Behavior

synthesis fail

povik commented 6 months ago

Minimal reproducer:

module top;
    wire [7:0] w = {"a", (1 ? "b" : "cc")};
endmodule
povik commented 6 months ago

Possibly relevant: https://github.com/YosysHQ/yosys/commit/891e4b5b0d16a537e6d1ca0ecbc1ac3bafccecef

DeLoSoCode commented 6 months ago

Minimal reproducer:

module top;
  wire [7:0] w = {"a", (1 ? "b" : "cc")};
endmodule

Thank you very much for your help

Khrig commented 3 months ago

Hi, I had a look round this and see that in simplification (const folding) of the AST for the ternary operator, the 'b' in the example above is expanded to the same size as the "cc" by appending a 'NULL' to the start of the string. Therefore the folded ternary expression is an AST_CONSTANT with value 0b00000000 01100010 or "NULL, b". This constant is still marked as a string with the "is_string" field. When the concatenation is simplified, the 'a' is inserted to the start before being passed to mkconst_str(). mkconst_str removes the 'NULL' due to the implementation of decode_string(), resulting in the error message shown.

I think this would be fixed by setting the is_string as false when a string has to be expanded in a ternary expression. let me know if I should make a PR with this change.

Its my first time trying to contribute to open source so not sure on the process here :) .

Thanks, Chris