zachjs / sv2v

SystemVerilog to Verilog conversion
BSD 3-Clause "New" or "Revised" License
497 stars 50 forks source link

Generate region standard compliance fixes #239

Open dwRchyngqxs opened 1 year ago

dwRchyngqxs commented 1 year ago

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 commented 1 year ago

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

dwRchyngqxs commented 1 year ago

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 commented 1 year ago

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.

zachjs commented 11 months ago

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?

dwRchyngqxs commented 11 months ago

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.

zachjs commented 10 months ago

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.