firrtl icon indicating copy to clipboard operation
firrtl copied to clipboard

Invalid Verilog if input IR is improperly inlined

Open seldridge opened this issue 4 years ago • 3 comments

The FIRRTL compiler correctly respects self-determined and context-determined widths when inlining expression. However, if an expression is already inlined, it won't be broken back out and you can get buggy Verilog.

There's a full example in this comment.

If you start with the following circuit:

circuit Foo :
  module Foo :
    input a : UInt<1>
    input inp_6 : UInt<13>
    output tmp13 : SInt<14>
    wire tmp11 : UInt<14>
    tmp11 <= add(inp_6, UInt<12>("h1"))
    node _tmp13_T_1 = mux(a, asSInt(tmp11), asSInt(UInt<13>("h1")))
    tmp13 <= _tmp13_T_1

This will inline tmp11 into:

module Foo(
  input         a,
  input  [12:0] inp_6,
  output [13:0] tmp13
);
  assign tmp13 = a ? $signed(inp_6 + 13'h1) : $signed(14'sh1);
endmodule

However, if you start with this un-inlined:

circuit Foo :
  module Foo :
    input a : UInt<1>
    input inp_6 : UInt<13>
    output tmp13 : SInt<14>
    wire tmp11 : UInt<14>
    tmp11 <= add(inp_6, UInt<12>("h1"))
    node x = asSInt(tmp11)
    node _tmp13_T_1 = mux(a, x, asSInt(UInt<13>("h1")))
    tmp13 <= _tmp13_T_1

It won't be inlined:

module Foo(
  input         a,
  input  [12:0] inp_6,
  output [13:0] tmp13
);
  wire [13:0] x = inp_6 + 13'h1;
  assign tmp13 = a ? $signed(x) : $signed(14'sh1);
endmodule

For safety, expressions may need to be broken out into nodes during parsing. However, this bug should be impossible to hit with the current way that Chisel emits CHIRRTL.

Checklist

  • [x] Did you specify the current behavior?
  • [x] Did you specify the expected behavior?
  • [x] Did you provide a code example showing the problem?
  • [x] Did you describe your environment?
  • [x] Did you specify relevant external information?

What is the current behavior?

See above.

What is the expected behavior?

You should get the second circuit if you input the first circuit.

Steps to Reproduce

Compile the above code with 1.4.1.

Your environment

1.4.1

External Information

See discussion on: https://github.com/llvm/circt/issues/394

seldridge avatar Jan 11 '21 06:01 seldridge

This was discussed during the Chisel dev meeting today. The conclusion is that this is an expression that should be split out by SplitExpressions, but is not (for some reason).

seldridge avatar Jan 11 '21 19:01 seldridge

Or possibly it's being incorrectly re-inlined in

jackkoenig avatar Jan 11 '21 19:01 jackkoenig

#2038 should fix this. The above input will now generate:

module Foo(
  input         a,
  input  [12:0] inp_6,
  output [13:0] tmp13
);
  wire [13:0] _GEN_0 = inp_6 + 13'h1;
  assign tmp13 = a ? $signed(_GEN_0) : $signed(14'sh1);
endmodule

seldridge avatar Jan 12 '21 18:01 seldridge