systemverilog compilation problems

[peter.t...@gmail.com]

Having generated SV via clash, I am getting errors with "verilator" (which I am a novice at, and which turns SV into  C++ for further compilation with gcc). The errors are the same whichever command line options I try for verilator, not that I know what the right ones are. I have tried specifying every version of SV supported by verilator, for example, ranging from 2005 to 2017 standards. I would welcome advice:

% verilator -ITest/TestBench --cc --exe --build -j TestBench_types TestBench sim_main.cpp

%Error: Test/TestBench/TestBench_fpu.sv:1192:17: Can't find definition of variable: '__VOID__' 1192

assign \x' = __VOID__;

It's complaining about the systemverilog. Should that be "VOID"?

%Error: Test/TestBench/TestBench_fpu.sv:1334:22: Can't find definition of variable: 'c$case_alt_5' : ... Suggested alternative: 'c$ds2_case_alt_5' 1334

assign result_9 = (c$case_alt_5);

^~~~~~~~~~~~

It's quite right. That variable is not defined. The ds2_ variant is defined higher up.

// KPU/Float.hs:244:1-13

TestBench_types::Tup3_36 c$ds2_case_alt_5;

Anything going on here?

PTB

[peter.t...@gmail.com]

I've generated verilog instead of systemverilog now, and there are far fewer errors. Just this one, basically:

%Error: Test/TestBench/TestBench_dcache_unit.v:4957:16: syntax error, unexpected packed, expecting IDENTIFIER or do or final or randomize
 4957 |     wire [1:0] packed;
      |                ^~~~~~

A variable called "packed", or a missing variable name.

 %Error: Test/TestBench/TestBench_dcache_unit.v:4974:12: syntax error, unexpected packed
 4974 |     assign packed = r;
      |            ^~~~~~
%Error: Test/TestBench/TestBench_dcache_unit.v:4977:44: syntax error, unexpected packed, expecting TYPE-IDENTIFIER
 4977 |                                           ,packed} : {64'sd0,2'bxx};
      |                                 

It's not a name I've used in the Clash source code.

    wire [1:0] r;
    wire [1:0] packed;                      <-- here is line 4957
    wire [65:0] result_50;
    wire signed [63:0] sc;
    wire [1:0] packedFields;

It looks like a bad choice of name by the generator.

I'll change it!

PTB

[Christiaan Baaij]

For technical reasons the Clash compiler cannot error out on certain things it doesn't want to render.

In that case it emits __VOID__, indicating that something has gone wrong.

One of the reasons it might not want to render something is when that something is of a type that needs zero-bits (e.g. BitVector 0, (), Vec 0 a, etc.)

Clash tries to filter out zero-bit values as much as possible, but there's one place us compiler developers (and potential users of advanced features of Clash) need to take extra care: primitive blackbox definitions.

In blackbox code we need to check whether arguments could be zero-bits explicitly, and if we forget to do that we could get erroneous code as you experienced.

It would be interesting to know what the net declaration for \x'  looks like.

Also, while the zero-bit thing is the most likely culprit, it is not necessarily the only culprit for Clash rendering __VOID__.

To help you narrow down a minimal example, you can compile your clash designs with the `-g` flag.

This will emit more line number hints in the generated code, e.g. when I compile the FIR code in the clash examples directory (https://github.com/clash-lang/clash-compiler/blob/master/examples/FIR.hs)

  cabal run clash -- --systemverilog -g examples/FIR.hs -fclash-clear -fclash-no-cache

the generated verilog will contain lines such as:

  // examples/FIR.hs:30:1-61
  // examples/FIR.hs:30:36-61
  assign x = c$x_app_arg;

which gives an indication that that line of generated code is somehow linked to this piece of haskell code (https://github.com/clash-lang/clash-compiler/blob/master/examples/FIR.hs#L30)

  topEntity = exposeClockResetEnable (fir (2:>3:>(-2):>8:>Nil))

Sometimes those line numbers include both declaration and use sites of variables.

Also, Clash tends to mangle and drop line number hints, so they're not super accurate; but they should give you some help in narrowing down a minimal example.

--
You received this message because you are subscribed to the Google Groups "Clash - Hardware Description Language" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clash-languag...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/clash-language/69a7fd3a-a194-4b7d-a348-d2b0dc949985n%40googlegroups.com.

[Christiaan Baaij]

Ah, but `packed` is not a Verilog-2001 reserved keyword: https://sutherland-hdl.com/pdfs/verilog_2001_ref_guide.pdf#page=7

It is however a SystemVerilog-2005 reserved keyword: https://www.intel.com/content/www/us/en/programmable/quartushelp/17.0/mapIdTopics/jka1465580561132.htm

And I guess since the 2012 merger of the standards also a (System)Verilog-2012 reserved keyword.

Perhaps you can instruct the tool you're using that the Verilog that's being generated is adhering to the Verilog-2001 standard?

Otherwise we have to extend Clash' notion of reserved keywords for the Verilog it emits: https://github.com/clash-lang/clash-compiler/blob/a737df6fac6dc79e7f972a4b50b115e3661ca13b/clash-lib/src/Clash/Netlist/Id/Verilog.hs#L22-L39

(Ignore that it says Verilog-2005 keywords, that's just the latest "stand-alone" Verilog standard, the Verilog Clash emits definitely adheres to the 2001 standard)

--
You received this message because you are subscribed to the Google Groups "Clash - Hardware Description Language" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clash-languag...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/clash-language/9e6744c1-dd7c-4bf8-b0dd-e84de00eae5fn%40googlegroups.com.

[peter.t...@gmail.com]

OK, thanks. Zero bits is plausible, but I had already looked for that in the source code and didn't see it. I'll look harder. I definitely grepped for "()" without success. I will be able to find the cause, I'm sure.

The other errors might be consequents. I have now fixed the first errors (above) out of the verilog generation and the same ones are now showing up lower down in the list there too.

% verilator -ITest/TestBench --cc --exe --build -j TestBench c_main.cpp
%Error: Test/TestBench/TestBench_fpu.v:1225:17: Can't find definition of variable: '__VOID__'
 1225 |   assign \x'  = __VOID__;
      |                 ^~~~~~~~

...

%Error: Test/TestBench/TestBench_fpu.v:1369:22: Can't find definition of variable: 'c$case_alt_5'
                                              : ... Suggested alternative: 'c$ds2_case_alt_5'
 1369 |   assign result_9 = (c$case_alt_5);
      |                      ^~~~~~~~~~~~

Etc.

Thanks again.

PTB

[peter.t...@gmail.com]

The __VOID__ is from the FPU file. I NOINLINEd practically everything and the __VOID__ disappeared. But the errors persist. The first is well-defined:

 

% verilator -IKPU/FPUa --cc --exe --build -j FPUa_types FPUa sim_main.cpp
%Error: KPU/FPUa/FPUa_mydecodeFloat.sv:17:17: Can't find definition of variable: 'x'
                                            : ... Suggested alternative: 'x''
   17 |   assign \x'  = x;
      |                 ^

and comes from this Clash source:

-- sign, exponent, significand without leading bit     
mydecodeFloat :: Float -> (Bool,Int8,Word23)
mydecodeFloat x     = (zSign,zExp,zFrac)
                where x' = pack x :: BitVector 32
                      zSign = unpack( slice d31 d31 x')      :: Bool
                      zExp  = unpack( slice d30 d23 x')      :: Int8
                      zFrac = unpack( slice d22 d0  x')      :: Word23

which has produced this module:

`timescale 100fs/100fs
module FPUa_mydecodeFloat
    ( // No inputs

      // Outputs
      output FPUa_types::Tup3_0 result
    );
  // KPU/Float.hs:138:23-49
  logic [31:0] \x' ;
  logic [0:0] bv;

  // KPU/Float.hs:138:23-49
  // KPU/Float.hs:138:28-49
  assign \x'  = x;

  // KPU/Float.hs:(137,1)-(141,70)
  assign result = {bv == 1'b1
                  ,$signed((\x' [30 : 23]))
                  ,(\x' [22 : 0])};

  assign bv = \x' [31 : 31];


endmodule

PTB

[Christiaan Baaij]

Yeah... we should really figure out how to properly raise an error message here.

The "problem" is that Clash doesn't want to render HDL for things of type `Float` (and `Double`) unless you compile your designs with `-fclash-float-support`

To view this discussion on the web visit https://groups.google.com/d/msgid/clash-language/a0b94642-6024-4e40-94b2-20e0496744c6n%40googlegroups.com.

[peter.t...@gmail.com]

Great. I don't need Float except for convenient display so I might as well use FLOAT = BitVector 32 instead. Meanwhile I have tried the -fclash-float-support flag and that leads to lots  of warnings:

% verilator -IKPU/FPUa --cc --exe --build -j FPUa_types FPUa sim_main.cpp

%Warning-LITENDIAN: KPU/FPUa/FPUa_types.sv:169:90: Little bit endian vector: MSB < LSB of bit range: 0:24
                                                 : ... In instance FPUa
  169 |   function automatic array_of_25_logic_vector_1 array_of_25_logic_vector_1_from_lv(logic [0:24][0:0] i);
      |                                                                                          ^
                    ... Use "/* verilator lint_off LITENDIAN */" and lint_on around source to disable this message.
%Warning-LITENDIAN: KPU/FPUa/FPUa_types.sv:133:90: Little bit endian vector: MSB < LSB of bit range: 0:63
                                                 : ... In instance FPUa
  133 |   function automatic array_of_64_logic_vector_1 array_of_64_logic_vector_1_from_lv(logic [0:63][0:0] i);
      |                                                                                          ^

which may or may not be significant.  It's running on an athlon. 

Presumably it wants [24:0] instead of [0:24], so maybe yes, there is some signficance!

I'll go the FLOAT direction.

Thanks

PTB

[peter.t...@gmail.com]

Still not sure whether the above warns are harmless or not - probably harmless. I think it's intentional and the generated code is colliding with software engineering good practice.

After turning those warnings off I am getting more, and these definitely are harmless, and yet also significant:

%Warning-WIDTH: KPU/FPUa/FPUa_myAddFloatPos.sv:87:50: Bit extraction of var[24:0] requires 5 bit index, not 64 bits.
                                                    : ... In instance FPUa.FPUa_fpu___05Fclash_internal.FPUa_fpu_sub_c__024ds3_case_alt_11.FPUa_sub_f_result_0.FPUa_mySubFloat_c__024a1_app_arg_1.FPUa_myAddFloat_c__024case_alt.FPUa_myAddFloatPos_c__024myAddFloatPosOut
   87 |   assign c$myAddFloatPos_$jOut_app_arg = ((c$bv_0[(64'sd24)]) == (1'b1)) ? ($signed(result_1.Tup2_6_sel0) + 8'sd1) : $signed(result_1.Tup2_6_sel0);

I think it's complaining about testBit foo 24. It notices that 24 is an Int, which is 64-bit: 64'sd24 (yep!). Nothing wrong, except it (verilator) thinks it's silly to use a 64-bit number to index a 24-bit number. It's right. A human author in systemverilog probably ought not to do that, yet it's forced by the Int argument to testBit and the code generator. I am going to get warnings all over the place from the Bits interface with this kind of generated code.

Should I turn off these warnings?  Or write a testBit_ which takes Enum i as an index? Better, an SNat -  only if that does not already exist!

PTB