YosysHQ / yosys

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

Preservation of escaped identifier #1676

Open towoe opened 4 years ago

towoe commented 4 years ago

SystemVerilog allows escaped identifiers in Section 5.6.1. The rule is that they must start with an backslash and end with a white space. All printable ASCII characters are included in the name.

Yosys can read those identifiers, but when using write_ilang or write_smt2 there is a problem if the identifiers are not in the top level of the hierarchy. This is because the path to the identifier is stored without the direct backslash of the identifier.

I attached a simple example which will generate a .ilang and .smt2 file of the design. Here is the gist of it.

module top ();
  bottom UUT ();
endmodule

module bottom ();
  reg \escaped[2] ;
endmodule

The signal is then referred to as \UUT.escaped[1]. I am not sure if this is correct or incorrect, or the result of some Yosys design choices. But if this identifier is used by SymbiYosys, which has the ability to create a Verilog test bench, the resulting identifier is UUT.escaped[1], which will be an invalid path for a simulation. I included the .smt2 file as this is the input to sby/Yosys and here both versions of the identifier are stored (\UUT.escaped[1] and UUT.escaped[1]). The one without any backslash is part of a Yosys specific comment which is used for the test bench creation. My assumption is that Yosys removes all backslashes and uses one in general as a prefix? This would be problematic if the original identifier needs to be recreated.

escaped_name.zip

whitequark commented 4 years ago

It looks to me that the problem here is that SymbiYosys doesn't have an understanding of hierarchy. AFAIU, in SV, the signal should be referred to as UUT.\escaped[1], i.e. with each level individually escaped. This is a problem because Yosys currently uses "in-band signaling" for hierarchy: an escaped identifier \UUT.escaped[1] and a hierarchical identifier UUT.\escaped[1] would result in the same flattened RTLIL name.

Does this match your understanding?

towoe commented 4 years ago

I also think that UUT.\escaped[1] would be the correct reference to the signal. Yes, I think Yosys will generate a flattened RTLIL name independent of the actual escaped signal name. So this would mean that the RTLIL name handling would need updating, in order to have the correct escaped hierarchy name at a later stage? This would sound like a major change.

whitequark commented 4 years ago

Yes. Unless @cliffordwolf has something better in mind here, I think IdString should be changed to be a list of identifiers rather than a single identifier. It seems very invasive and laborous.

I understand that we could use a workaround that uses another in-band character to signal hierarchy boundaries but I personally find that an unacceptable solution.

whitequark commented 4 years ago

@towoe I believe that the commit https://github.com/YosysHQ/yosys/commit/fbb346ea91a04f2feaf6fa96770fe0cd57020e75 fixes this issue by attaching a hdlname attribute during flattening. Is there anything left to do, in your understanding?