verilog-to-routing / vtr-verilog-to-routing

Verilog to Routing -- Open Source CAD Flow for FPGA Research
https://verilogtorouting.org
Other
988 stars 380 forks source link

Odin Doesn't Support Syntax for Escaped Verilog Identifiers #2415

Open stefanpie opened 9 months ago

stefanpie commented 9 months ago

Hello, VTR devs. This is Stefan Abi-Karam from Georgia Tech, and I am running into some simple parsing errors when it comes to using Odin II on some Verilog designs. The core issue is that Odin II seems to support only a limited subset of allowable verilog syntax (just talking about "simple" 1995 Verilog, nothing new or SystemVerilog related), which excludes using backslashes for escaping identifiers. This causes some issues in my current flow when using Yosys before sending designs into Odin II. Please see the "2.7.1 Escaped identifiers" section of the 1995 Verilog standard (https://ieeexplore.ieee.org/document/803556) for more details and examples of what I am referring to.

My current flow looks as follows:

  1. Read a bunch of Verilog files into Yosys for a single design
  2. Do some simple cleanup in Yosys, nothing synthesis or optimization related.
  3. Write back out to a single Verilog file for the entire design.
  4. Read into Odin II for synthesis and technology mapping.

Yosys tends to produce Verilog that is generally good for Odin, except for the fact that it uses backslash to escape identifiers, which means that after the backslash, they can contain any of the printable ASCII characters in an identifier as long as a space follows the identifier.

Examples of escaping identifiers using backslash:

reg [63:0] \$5\new_ascii_instr[63:0] ;
reg [4:0] \$memwr$\cpuregs$/<redacted_local_file_path>/picorv32/picorv32.v:1339$17_ADDR ;
...
    if (\$auto$verilog_backend.cc:2184:dump_module$854 ) begin end
...

I have also seen this behavior with module names as well.

Even though the Parmys flow seems to be the way forward, I still think it's reasonable to use Odin in the meantime and expect it to handle escaping identifiers. Many other users and tools besides Yosys might also use this feature as well, especially if a tool is autogenerating some Verilog.

Expected Behaviour

Odin II should be able to parse Verilog identifiers that are escaped via a backslash according to the 1995 Verilog Standard.

Current Behaviour

Right now, Odin II cannot parse Verilog identifiers that are escaped with a backslash.

Some forms of this error from Odin II are shown below.

<redacted_local_file_path>test_vpr_flow0/picorv32_merged.v:5:1: warning[PARSE_TO_AST]:  error in parsing: (syntax error, unexpected vsFUNCTION, expecting vSYMBOL_ID or vSIGNED or vUNSIGNED or '[')
     3 | module picorv32(clk, resetn, trap, mem_valid, mem_instr, mem_ready, mem_addr, mem_wdata, mem_wstrb, mem_rdata, mem_la_read, mem_la_write, mem_la_addr, mem_la_wdata, mem_la_wstrb, pcpi_valid, pcpi_insn, pcpi_rs1, pcpi_rs2, pcpi_wr, pcpi_rd
     4 | , pcpi_wait, pcpi_ready, irq, eoi, trace_valid, trace_data);
     5 |   reg \$auto$verilog_backend.cc:2184:dump_module$854  = 0;
          ^~~~
     6 |   reg [4:0] \$0$memwr$\cpuregs$/<redacted_local_file_path>/picorv32/picorv32.v:1339$17_ADDR[4:0]$482 ;
     7 |   reg [31:0] \$0$memwr$\cpuregs$/<redacted_local_file_path>/picorv32/picorv32.v:1339$17_DATA[31:0]$483 ;
<redacted_local_file_path>test_vpr_flow0/picorv32_merged.v:13:1: warning[PARSE_TO_AST]:  error in parsing: (syntax error, unexpected vINT_NUMBER, expecting vSYMBOL_ID)
    11 |   reg \$0\alu_lts[0:0] ;
    12 |   reg \$0\alu_ltu[0:0] ;
    13 |   reg [31:0] \$0\alu_out[31:0] ;
          ^~~~
    14 |   reg \$0\alu_out_0[0:0] ;
    15 |   reg \$0\alu_out_0_q[0:0] ;
<redacted_local_file_path>test_vpr_flow0/picorv32_merged.v:1208:1: warning[PARSE_TO_AST]:  error in parsing: (syntax error, unexpected vsFUNCTION, expecting vSYMBOL_ID or '{' or '#')
  1206 |   assign \decoded_rs2_$memrd_ADDR_DATA  = cpuregs[decoded_rs2];
  1207 |   assign \decoded_rs1_$memrd_ADDR_DATA  = cpuregs[decoded_rs1];
  1208 |   assign \$and$/<redacted_local_file_path>/picorv32/picorv32.v:1958$673_Y  = { \$1\next_irq_pending[31:0] [31:3], \$16\next_irq_pending[2:2] , \$1\next_irq_pending[31:0] [1:0] } & 32'd4294967295;
          ^~~~
  1209 |   assign \alu_eq_$logic_not_A_Y  = ! alu_eq;
  1210 |   assign \alu_lts_$logic_not_A_Y  = ! alu_lts;
<redacted_local_file_path>test_vpr_flow0/picorv32_merged.v:1849:1: warning[PARSE_TO_AST]:  error in parsing: (syntax error, unexpected vsFUNCTION)
  1847 |   assign \trap_$logic_or_B_A  = ! resetn;
  1848 |   always @* begin
  1849 |     if (\$auto$verilog_backend.cc:2184:dump_module$854 ) begin end
          ^~~~
  1850 |     casez (1'h0)
  1851 |       default:
error[PARSE_TO_AST]:  Parser found (165) errors in your syntax, exiting

Possible Solution

Looking at the parsing code for Odin in /odin_ii/src/verilog/verilog_flex.l, it seems that the parsing code could be modified to support this as follows.

vWORD [[:alpha:]_][[:alnum:]_$]*
vESC  \\[ -~]+

/* later in the code... */

<INITIAL>{vWORD}(\.{vWORD})*            { MP(); yylval.id_name = vtr::strdup(yytext); return vSYMBOL_ID; }
<INITIAL>{vESC}[[:space:]]             { MP(); yylval.id_name = vtr::strndup(yytext+1, yyleng-2); return vSYMBOL_ID; }

I am not a C++, Bison, or flex expert, but this seems like one possible way to approach the problem after some initial research.

The other "solution" is just to use Parmys as the way forward. I have not tested this yet, as getting a build from a source working for me is still a work in progress on my limited-sudo-access research server. I would assume this works since it is a plugin for Yosys, and Yosys will happily parse and use this escaped identifier syntax everywhere. This would also fit into my EDA flow anyway.

Steps to Reproduce

You can reproduce this with a simple test.v Verilog design and Odin II:

module adder(
    input [3:0] a,
    input [3:0] b,
    output [3:0] sum
);

assign \%3452345()^&%^&5sum = a + b;

endmodule

I just call Odin II as follows:

odin_II -c EArch.xml -V test.v -o test.out.blif

I am using the EArch.xml from /vtr_flow/arch/timing/EArch.xml.

Context

This is mainly causing issues in the EDA flow that I am currently using, as described above, under the assumption that I don't have Parmys. In general, I want to be able to do some simple preprocessing and cleanup of my design in another tool like Yosys before passing it into Odin, and I want to let upstream tools generate Verilog for Odin that supports escaped identifiers.

Using Yosys, I tried various techniques and combinations of commands to prevent the exported verilog from having escaped identifiers, but none worked.

Your Environment

I am using a build of VTR, Odin, and Yosys packages as conda packages form from the litex-hub (https://github.com/litex-hub/litex-conda-packages + https://anaconda.org/LiteX-Hub).

odin_ii                   8.0.0_3390_g4acad0fb5 20210302_150524    litex-hub
vtr-optimized             8.0.0_6959_ga7fae8fb2 20230131_213614    litex-hub
yosys                     0.33_11_g31ee566ec 20230724_080446_py310    litex-hub
stefanpie commented 9 months ago

I wanted to follow up on this pull request after some more experimenting:

  1. There are other small Verilog syntax edge cases not supported by Odin that are defined in the Verilog 1995 standard.
  2. I made a Verilog "preprocessing" Python module as a quick workaround that works for now.

Other Verilog Edge Cases Odin II does not support spaces in integer literals. For example, the following integer literals will cause errors in Odin II.

32'h 0000_0000,
32'h ffff_ffff,
32'h 0000_0010,

However, if you remove the space from the integer literals, as shown below, Odin II can parse the Verilog without throwing those specific errors.

32'h0000_0000,
32'hffff_ffff,
32'h0000_0010,

For more details and examples, see section "2.5.1 Integer constants" of the Verilog 1995 standard (https://ieeexplore.ieee.org/document/803556).

Workaround Python Preprocessing Module To workaround both the escaped identifier and integer literal parsing bugs in Odin II, I wrote a Python module that can "preprocess" Verilog source to transform it into Verilog that Odin II can currently parse. This seems to address the two main issues I am having at the moment, and I wanted to share this in case others come across this issue. See the attached zip file for the module code: verilog_preprocessor.zip. I use Python 3.10 with type annotations. The module only uses standard library imports and should work standalone.