Bluespec-emitted Verilog makes Yosys unhappy

[thoughtpolice]

The Bluespec compiler emits Verilog which makes Yosys somewhat unhappy:

1.3. Executing Verilog-2005 frontend: /tmp/yosys-bsv-v-QxaOnJ/keccak.v
Parsing Verilog input from `/tmp/yosys-bsv-v-QxaOnJ/keccak.v' to AST representation.
Warning: Found one of those horrible `(synopsys|synthesis) parallel_case' comments.
Yosys does support them but it is recommended to use Verilog `parallel_case' attributes instead!
Warning: Found one of those horrible `(synopsys|synthesis) translate_off' comments.
Yosys does support them but it is recommended to use `ifdef constructs instead!
Generating RTLIL representation for module `\keccak'.
Note: Assuming pure combinatorial block at /tmp/yosys-bsv-v-QxaOnJ/keccak.v:554 in
compliance with IEC 62142(E):2005 / IEEE Std. 1364.1(E):2002. Recommending
use of @* instead of @(...) for better match of synthesis and simulation.
Note: Assuming pure combinatorial block at /tmp/yosys-bsv-v-QxaOnJ/keccak.v:568 in
compliance with IEC 62142(E):2005 / IEEE Std. 1364.1(E):2002. Recommending
use of @* instead of @(...) for better match of synthesis and simulation.
Note: Assuming pure combinatorial block at /tmp/yosys-bsv-v-QxaOnJ/keccak.v:590 in
compliance with IEC 62142(E):2005 / IEEE Std. 1364.1(E):2002. Recommending
use of @* instead of @(...) for better match of synthesis and simulation.
Note: Assuming pure combinatorial block at /tmp/yosys-bsv-v-QxaOnJ/keccak.v:1357 in
compliance with IEC 62142(E):2005 / IEEE Std. 1364.1(E):2002. Recommending
use of @* instead of @(...) for better match of synthesis and simulation.
Note: Assuming pure combinatorial block at /tmp/yosys-bsv-v-QxaOnJ/keccak.v:1366 in
compliance with IEC 62142(E):2005 / IEEE Std. 1364.1(E):2002. Recommending
use of @* instead of @(...) for better match of synthesis and simulation.
Note: Assuming pure combinatorial block at /tmp/yosys-bsv-v-QxaOnJ/keccak.v:1440 in
compliance with IEC 62142(E):2005 / IEEE Std. 1364.1(E):2002. Recommending
use of @* instead of @(...) for better match of synthesis and simulation.
Successfully finished Verilog frontend.

---

The first warning is triggered by this (parallel_case):

  begin
    case (1'b1) // synopsys parallel_case
      WILL_FIRE_RL_permute: first_block$D_IN = counter_block == 6'd32;
      init_enable: first_block$D_IN = 1'd0;
      WILL_FIRE_RL_do_go: first_block$D_IN = 1'd1;
      default: first_block$D_IN = 1'bx /* unspecified value */ ;
    endcase
  end

Fix: use Verilog-standard (* parallel_case *) instead:

  begin
    (* parallel_case *)
    case (1'b1)
      WILL_FIRE_RL_permute: first_block$D_IN = counter_block == 6'd32;
      init_enable: first_block$D_IN = 1'd0;
      WILL_FIRE_RL_do_go: first_block$D_IN = 1'd1;
      default: first_block$D_IN = 1'bx /* unspecified value */ ;
    endcase
  end

---

Second (translate_off):

  // synopsys translate_off
  `ifdef BSV_NO_INITIAL_BLOCKS
  `else // not BSV_NO_INITIAL_BLOCKS
  initial
  begin
    counter_block = 6'h2A;
    counter_nr_rounds = 5'h0A;
    first_block = 1'h0;
    first_round = 1'h0;
    permutation_computed = 1'h0;
    reg_data =
        1600'hAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA;
    theta_parity_reg = 5'h0A;
  end
  `endif // BSV_NO_INITIAL_BLOCKS
  // synopsys translate_on

Fix: add ifdef YOSYS to chain and scope properly. This helps scope the translation to simulators.

  `ifdef BSV_NO_INITIAL_BLOCKS
  `elsif YOSYS
  `else // not BSV_NO_INITIAL_BLOCKS
  // synopsys translate_off
  ...
  // synopsys translate_on
  `endif // BSV_NO_INITIAL_BLOCKS

---

Third (combinatorial block):

  // register first_block
  always@(WILL_FIRE_RL_permute or
          counter_block or init_enable or WILL_FIRE_RL_do_go)
  begin
    case (1'b1) // synopsys parallel_case
      WILL_FIRE_RL_permute: first_block$D_IN = counter_block == 6'd32;
      init_enable: first_block$D_IN = 1'd0;
      WILL_FIRE_RL_do_go: first_block$D_IN = 1'd1;
      default: first_block$D_IN = 1'bx /* unspecified value */ ;
    endcase
  end
  assign first_block$EN =
             WILL_FIRE_RL_permute || init_enable || WILL_FIRE_RL_do_go ;

Fix: just use always @* for combinational blocks, since there aren't any clock edges used in the sensitivity list:

  // register first_block
  always @*
  begin
    ...
  end

[bpfoley]

Could you give us some bluespec reproducers for these please?

1. If Synopsys tools now support the standard Verilog notation for (* parallel_case *) it should be easy to change BSC just to emit that unconditionally.
2. I suspect it might be better to wrap the translate comments in ifdef DESIGNCOMPILER
3. This sounds about right. But IANAVE (Verilog expert :) )

[thoughtpolice]

I haven't cooked up an example but realistically you can probably feed just about any design to yosys and reproduce these, literally anything. In particular the initial block one will happen in almost any non-trivial design (anything that uses a register), because the compiler will (by definition) insert the initial block to initialize them in simulation.

In my understanding, yes, Synopsys does support standardized Verilog 2001 attributes (for everything it supports attributes for), the comment style is just the pre-standardized holdover. You can find this in various Synplify manuals, etc. So I think it's safe to just use them.

I do have patches that (confirmed) fix issues 1 and 2. Perhaps the more pressing issue wrt 2 is that attributes were only added with standard Verilog 2001, but BSV seems to generally target the weaker feature set of Verilog 95. Frankly I think this is probably fine to ignore, for the most part, in the name of minimizing tool-specfic behavior/support.

[quark17]

BSC has a v95 flag, that is off by default (so the default behavior is not constrained to Verilog 95). I would think that it's OK to change BSC to emit anything that's legal Verilog 2001, as long as an alternative is used when the v95 flag is given.

[thoughtpolice]

Can we flip that on its head and ask if we have to preserve Verilog 95 support? What if we targeted a subset of Verilog 2001 (and say, kept the -v95 flag as a no-op that emitted a warning)? In particular 95 would definitely make the implementation more complicated; to my knowledge -v95 is only used in one place in the compiler (something about the way module ports get emitted) but otherwise isn't "threaded" through into the Verilog pretty printer in Verilog.hs. So choosing how to print a module at runtime might be a very invasive change, I'm not sure OTTOMH... It seems rather overkill to do a 1000 line code change for a single warning in a single synthesis tool to fix one thing that we could just stick with.

More generally, the problem is really that parallel_case and full_case just aren't well specified by Verilog itself, so whether or not any of these even applies is a crap shoot. I mean, I'm not a Verilog master, but it's completely fine if any synthesis tool just flat-out ignored attributes and // synopsys comments, right? It doesn't have to respect them at all. Verilog 2001 is only an improvement over Verilog 95 here in the sense that attributes themselves are formalized, but the semantics of what the attributes imply is up in the air. SystemVerilog got this right by having separate actual keywords for these different behaviors, at least...

In general it's all kind of a crapshoot I feel, without doing extremely laborous review of every synthesis tool on earth. "Verilog 95" or "Verilog 2001" support often mean incredibly vague things without digging into it; for instance Vendors (like Intel) say they "only" support VHDL 94 or whatever, even though they do support almost all of the synthesizable subset of newer VHDL standards. They say they "only" support VHDL 94 though, because they don't support all of the simulation constructs (meanwhile vendors like Cadence or whatever support "the entire" VHDL standard, including all simulation constructs, but I'm guessing many of them don't matter). Despite that, strict Verilog 95 is probably only enforced or required by the oldest, creakiest, worst-est of EDA tools at this point.

---

Also: Targeting newer Verilog would also be nice so we could clean up other things; the way the compiler does old-school port rendering is actually a nitpick I have, because I absolutely hate having to look for wire declarations to see if which ports are inputs or outputs (i.e. the difference between module t(clk, resetn); vs module t(input clk, input resetn). I believe that's also a 2001 feature. This is obviously less of an issue if you have the Bluespec code in front of you, but if I want to e.g. produce Verilog code from Bluespec and give the Verilog it to people, it's very annoying for them to have to look at the module declarations and see this. Maybe this doesn't matter, but it would be nice if Bluespec could emit beautiful code, too. :)