sv2v icon indicating copy to clipboard operation
sv2v copied to clipboard

Generate region standard compliance fixes

Open dwRchyngqxs opened this issue 2 years ago • 6 comments

Standard compliance fixes (Checked against Verilog 1995/2001/2005 and SystemVerilog 2017 A.4.2):

Do not accept generate regions inside other generate regions; this is now forbidden

module top();
  generate
    generate
    endgenerate
  endgenerate
endmodule

Do not print lone semicolon in generate region; this cannot be printed anymore

module top();
  generate
    ;
  endgenerate
endmodule

Print a block in for generate to be compatible with Verilog 2001

module top();
  for (...) begin
    ...
  end
endmodule

EDIT: Verilog standard are complicated because the syntax of a standard does not cover everything from the previous standard. EDIT2: Added comments to explain why begin/end blocks are added in if and for.

dwRchyngqxs avatar May 23 '23 11:05 dwRchyngqxs

I accidentally prevented attributes on conditional and loop generate construct. I'm going to fixing that before going out of draft.

dwRchyngqxs avatar May 23 '23 20:05 dwRchyngqxs

Apparently attributes on generate regions are allowed in Verilog 2001, but Verilog 2005 is not a superset of Verilog 2001 so I didn't notice it before reading both standards... So in the end it's better to parse it but not print it. There is still somethings to do in order to prevent an escalation from NonGenerateModuleItem to ModuleItem by putting an attribute. I guess I will convert this PR into fixing that bug and other non standard generate stuff printing.

dwRchyngqxs avatar May 23 '23 21:05 dwRchyngqxs

Ok, I think this is now correct. Only tests are left. EDIT: It is incorrect, the removed begin/end in if statements are actually useful for the dangling else problem.

dwRchyngqxs avatar May 24 '23 10:05 dwRchyngqxs

I just pushed some revisions that should preserve most of the intended changes while fixing the issues raised in the test cases. What do you think?

zachjs avatar Aug 01 '23 03:08 zachjs

I've read your changes. They solve what I was trying to solve, but correctly. Thank you for your help, I was unable to understand how sv2v handles generate regions and that was stalling the PR.

dwRchyngqxs avatar Aug 01 '23 10:08 dwRchyngqxs

I realized that my changes actually break applying attributes to generate items, so I'll have to work on this further. I am considering adding a GIAttr akin to MIAttr, or potentially a larger refactor combining GenItem into ModuleItem.

zachjs avatar Aug 09 '23 02:08 zachjs