Prepend Verilog globals to module AST #4656

jmi2k · 2024-10-11T19:23:09Z

What are the reasons/motivation for this change?

Fixes #4653. Further AST and RTLIL stages seem to be order-sensitive, and appending globals to the module children list did not work.

Explain how this is achieved.

Early during AST::process(), Verilog globals are prepended to the children list instead.

If applicable, please suggest to reviewers how they can test the change.

#4653 contains a test case that triggers an assertion, but works fine after applying this change.

Fixes YosysHQ#4653. Further AST and RTLIL stages seem to be order-sensitive, and appending globals to the module children list did not work.

jmi2k · 2024-10-11T23:12:20Z

This seems to have caused (or uncovered an issue with typedefd packed multidimensional arrays (in arrays03.sv). Commenting the affected cases unblocks the test suite up to this point.

[...]
Passed sign_array_query.ys
Passed size_cast.ys
Passed struct_access.ys
<<EOT:6: ERROR: syntax error, unexpected ';', expecting ATTR_BEGIN or TOK_INCREMENT or TOK_DECREMENT
Expected error pattern 'syntax error, unexpected ';', expecting ATTR_BEGIN or TOK_INCREMENT or TOK_DECREMENT' found !!!
Passed task_attr.ys
Passed typedef_across_files.ys
Passed typedef_const_shadow.ys
ERROR: Called with -verify and proof did fail!
make[1]: *** [run-test.mk:496: typedef_legacy_conflict.ys] Error 1
make[1]: Leaving directory '/home/jsanchez/Documents/Sources/yosys/tests/verilog'
make: *** [Makefile:898: test] Error 2

I will try to shed more light into this issue.

jmi2k · 2024-10-12T11:40:15Z

It seems like there's a mismatch in the resulting dimensions after simplification between declaring a 2D packed array of wires directly vs. using a typedef, where the former keeps the 2D dimensions but the latter collapses them into a single 1D range.

Example Verilog code used to test this behavior:

typedef logic [3:0][1:0] Matrix;

module Top;

`ifdef INLINE
	logic [3:0][1:0] matrix;
`else
	Matrix matrix;
`endif

	assign matrix = "a";

endmodule

Simplified AST when INLINE is defined:

Dumping AST after simplification:
    AST_MODULE <rtl/SoC.sv:3.1-13.10> [0x59d814aa2470] str='\Top' basic_prep
      AST_TYPEDEF <rtl/SoC.sv:0.0-0.0> [0x59d814a92ec0] str='\Matrix' basic_prep
        AST_WIRE <rtl/SoC.sv:0.0-0.0> [0x59d814a93010] logic basic_prep range=[7:0] dimensions=[3:0][1:0]
          AST_RANGE <rtl/SoC.sv:0.0-0.0> [0x59d814a93160] basic_prep range=[7:0] in_param
            AST_CONSTANT <rtl/SoC.sv:0.0-0.0> [0x59d814a93970] bits='00000000000000000000000000000111'(32) signed basic_prep range=[31:0] int=7 in_param
            AST_CONSTANT <rtl/SoC.sv:0.0-0.0> [0x59d814a936c0] bits='00000000000000000000000000000000'(32) signed basic_prep range=[31:0] in_param
      AST_WIRE <rtl/SoC.sv:8.9-8.15> [0x59d814aa2800] str='\matrix' logic basic_prep range=[7:0] dimensions=[7:0]
        ATTR \wiretype:
          AST_CONSTANT <rtl/SoC.sv:0.0-0.0> [0x59d814a95920] str='\Matrix' bits='01011100010011010110000101110100011100100110100101111000'(56) basic_prep range=[55:0] int=1953655160 in_param
        AST_RANGE <rtl/SoC.sv:0.0-0.0> [0x59d814a95570] basic_prep range=[7:0] in_param
          AST_CONSTANT <rtl/SoC.sv:0.0-0.0> [0x59d814a956c0] bits='00000000000000000000000000000111'(32) signed basic_prep range=[31:0] int=7 in_param
          AST_CONSTANT <rtl/SoC.sv:0.0-0.0> [0x59d814a957f0] bits='00000000000000000000000000000000'(32) signed basic_prep range=[31:0] in_param
      AST_ASSIGN <rtl/SoC.sv:11.9-11.21> [0x59d814aa2a60] basic_prep
        AST_IDENTIFIER <rtl/SoC.sv:11.9-11.15> [0x59d814aa25a0 -> 0x59d814aa2800] str='\matrix' basic_prep in_lvalue
        AST_CONSTANT <rtl/SoC.sv:11.19-11.21> [0x59d814aa26d0] str='a' bits='01100001'(8) basic_prep range=[7:0] int=97
--- END OF AST DUMP ---

Simplified AST when INLINE is not defined:

Dumping AST after simplification:
    AST_MODULE <rtl/SoC.sv:3.1-13.10> [0x58a285b17360] str='\Top' basic_prep
      AST_TYPEDEF <rtl/SoC.sv:0.0-0.0> [0x58a285b17c20] str='\Matrix' basic_prep
        AST_WIRE <rtl/SoC.sv:0.0-0.0> [0x58a285b17d70] logic basic_prep range=[7:0] dimensions=[3:0][1:0]
          AST_RANGE <rtl/SoC.sv:0.0-0.0> [0x58a285b17ec0] basic_prep range=[7:0] in_param
            AST_CONSTANT <rtl/SoC.sv:0.0-0.0> [0x58a285b18670] bits='00000000000000000000000000000111'(32) signed basic_prep range=[31:0] int=7 in_param
            AST_CONSTANT <rtl/SoC.sv:0.0-0.0> [0x58a285b183c0] bits='00000000000000000000000000000000'(32) signed basic_prep range=[31:0] in_param
      AST_WIRE <rtl/SoC.sv:6.19-6.25> [0x58a285b27fa0] str='\matrix' logic basic_prep range=[7:0] dimensions=[3:0][1:0]
        AST_RANGE <rtl/SoC.sv:0.0-0.0> [0x58a285b28200] basic_prep range=[7:0] in_param
          AST_CONSTANT <rtl/SoC.sv:0.0-0.0> [0x58a285b28350] bits='00000000000000000000000000000111'(32) signed basic_prep range=[31:0] int=7 in_param
          AST_CONSTANT <rtl/SoC.sv:0.0-0.0> [0x58a285b28480] bits='00000000000000000000000000000000'(32) signed basic_prep range=[31:0] in_param
      AST_ASSIGN <rtl/SoC.sv:11.9-11.21> [0x58a285b27ac0] basic_prep
        AST_IDENTIFIER <rtl/SoC.sv:11.9-11.15> [0x58a285b27d20 -> 0x58a285b27fa0] str='\matrix' basic_prep in_lvalue
        AST_CONSTANT <rtl/SoC.sv:11.19-11.21> [0x58a285b27bf0] str='a' bits='01100001'(8) basic_prep range=[7:0] int=97
--- END OF AST DUMP ---

Notice how the dimension property is [3:0][1:0] when declaring the type as a 2D packed array directly, but [7:0] when going through a typedef.

My guess is that the code below line 1944 of frontends/ast/simplify.cc is not right.

Honestly, I think I'm going to need some help here, so if somebody knows the internals better than me (so, knows them at all) I would appreciate some guidance.

jmi2k requested a review from zachjs as a code owner October 11, 2024 19:23

Prepend Verilog globals to module AST

5c62838

Fixes YosysHQ#4653. Further AST and RTLIL stages seem to be order-sensitive, and appending globals to the module children list did not work.

jmi2k force-pushed the fix-verilog-globals branch from b70dbf0 to 5c62838 Compare October 11, 2024 19:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepend Verilog globals to module AST #4656

Prepend Verilog globals to module AST #4656

jmi2k commented Oct 11, 2024

jmi2k commented Oct 11, 2024

jmi2k commented Oct 12, 2024 •

edited

Loading

Prepend Verilog globals to module AST #4656

Are you sure you want to change the base?

Prepend Verilog globals to module AST #4656

Conversation

jmi2k commented Oct 11, 2024

jmi2k commented Oct 11, 2024

jmi2k commented Oct 12, 2024 • edited Loading

jmi2k commented Oct 12, 2024 •

edited

Loading