Generate Verilator-friendly verilog

[adamwalker]

Clash generates Verilog that Verilator has difficulty optimising. For example, given the module below:

module Mod where

import Clash.Prelude

count :: (HiddenClockReset dom gated sync, Num a) => Signal dom Bool -> Signal dom a
count inc = res
    where
    res = regEn 0 inc $ res + 1

cntrs 
    :: forall dom gated sync. (HiddenClockReset dom gated sync)
    => Signal dom Bool
    -> Vec 2 (Signal dom (BitVector 16))
cntrs v = c1 :> c2 :> Nil
    where
    c1 :: Signal dom (BitVector 16)
    c1 =  count v

    c2 :: Signal dom (BitVector 16)
    c2 =  regEn 0 v $ c2 + 1

----------------------------------------------------------
--Synthesis
----------------------------------------------------------
{-# ANN topEntity
  (Synthesize
    { t_name     = "mod"
    , t_inputs   = []
    , t_output   = PortProduct "" []
    }) #-}
topEntity clk rst = withClockReset clk rst (cntrs @System @Source @Synchronous)

Clash generates:

/* AUTOMATICALLY GENERATED VERILOG-2001 SOURCE CODE.
** GENERATED BY CLASH 0.99.3. DO NOT MODIFY.
*/
module Mod_count
    ( // Inputs
      input [1:0] \$d(%,%)  
    , input  inc 

      // Outputs
    , output wire [15:0] res 
    );
  reg [15:0] \#res_rec ;
  wire [1:0] \#res_app_arg ;

  // register begin
  always @(posedge \#res_app_arg [1] ) begin : Mod_count_register
    if (\$d(%,%) [0:0]) begin
      \#res_rec  <= 16'b0000000000000000;
    end else if (\#res_app_arg [0]) begin
      \#res_rec  <= (\#res_rec  + 16'b0000000000000001);
    end
  end
  // register end

  // clockGate begin 
  assign \#res_app_arg  = {\$d(%,%) [1:1],inc};
  // clockGate end

  assign res = \#res_rec ;
endmodule

And compilation with Verilator fails with these warnings:

%Warning-UNOPTFLAT: verilog/Mod/mod//Mod_count.v:13: Signal unoptimizable: Feedback to clock or circular logic: mod.Mod_count_app_arg.#res_app_arg
%Warning-UNOPTFLAT: Use "/* verilator lint_off UNOPTFLAT */" and lint_on around source to disable this message.
%Warning-UNOPTFLAT:      Example path: verilog/Mod/mod//Mod_count.v:13:  mod.Mod_count_app_arg.#res_app_arg
%Warning-UNOPTFLAT:      Example path: verilog/Mod/mod//Mod_count.v:16:  ASSIGNW
%Warning-UNOPTFLAT:      Example path: verilog/Mod/mod//Mod_count.v:16:  mod.Mod_count_app_arg.__Vsenitemexpr1
%Warning-UNOPTFLAT:      Example path: verilog/Mod/mod//Mod_count.v:16:  ACTIVE
%Warning-UNOPTFLAT:      Example path: verilog/Mod/mod//Mod_count.v:16:  ALWAYS
%Warning-UNOPTFLAT:      Example path: verilog/Mod/mod//Mod_count.v:13:  mod.Mod_count_app_arg.#res_app_arg
%Warning-UNOPTFLAT:      Example path: verilog/Mod/mod//Mod_count.v:26:  ASSIGNW
%Warning-UNOPTFLAT:      Example path: verilog/Mod/mod//Mod_count.v:13:  mod.Mod_count_app_arg.#res_app_arg
%Error: Exiting due to 1 warning(s)
%Error: Command Failed /usr/bin/verilator_bin -cc verilog/Mod/mod/mod.v --exe sim_main.cpp -Iverilog/Mod/mod/

Verilator was invoked with the command:

verilator -cc verilog/Mod/mod/mod.v -Iverilog/Mod/mod/ 

These errors can be suppressed and Verilator will then simulate the module without errors, but the C++ code it generates is suboptimal. This can slow down simulation a lot in larger designs. The root cause is that the clock signal is packed into #res_app_arg, along with another non-clock signal, and then unpacked again when it is used in the always block. If the code is modified to use the module input clock signal directly instead of going via the #res_app_arg array everything works fine. Also, if the clock goes via other intermediate signals without being packed with other non-clock signals, everything also works fine. Of course, there is nothing logically wrong with the Clash-generated Verilog, but it would be neat if Verilog generated by Clash could be efficiently simulated with Verilator.

[leonschoorl]

If you change HiddenClockReset dom gated sync to HiddenClock dom gated, HiddenReset dom sync does that make Verilator any happier?

[adamwalker]

Yep, that works around this specific issue, thanks. But, it exposes others. For example, the fold function compiles to code where elements of an array depend on other elements in that same array in the same cycle. That also makes Verilator unhappy.

[martijnbastiaan]

Now that we've merged:

• https://github.com/clash-lang/clash-compiler/pull/862
• https://github.com/clash-lang/clash-compiler/pull/839

We should check whether Verilotor is happy with the HDL we generate; maybe we could add it to the testsuite too.

[christiaanb]

@alex-mckenna Now that verilator is part of our test bench and CI, do you think we can close this?

[alex-mckenna]

Yes / no. The %Warning-UNOPTFLAT warning triggers for basically any design clash generates (certainly any non-trivial one). Changing Clash netlist to not produce such unoptimized HDL would speed up every simulation tool, and I think is worth doing. ~I do wonder if it should have it's own issue since the problem is not more specific than this issue.~ I do wonder if it should have it's own issue since there are other warnings which trigger for basically any design, and it might make sense to list them all in one place.