[FIRRTL] Missed Combinational Loop Detection

[seldridge]

The following circuit should trip combinational loop checking, but does not:

circuit Foo:
  module Foo:
    output b: UInt<1>

    b <= not(b)

This gets into HW dialect and crashes on malformed IR of a unary xor. 👀

%0 = comb.xor %0 : i1

Assertion failed: (size > 1 && "expected 2 or more operands"), function canonicalize, file CombFolds.cpp, line 1228.
Process 65409 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert
    frame #4: 0x0000000100be2fe6 firtool`circt::comb::XorOp::canonicalize(op=XorOp @ 0x00007ff7bfefd388, rewriter=0x00007ff7bfefdcc0) at CombFolds.cpp:1228:3
   1225 LogicalResult XorOp::canonicalize(XorOp op, PatternRewriter &rewriter) {
   1226   auto inputs = op.inputs();
   1227   auto size = inputs.size();
-> 1228   assert(size > 1 && "expected 2 or more operands");
   1229 
   1230   // xor(..., x, x) -> xor (...) -- idempotent
   1231   if (inputs[size - 1] == inputs[size - 2]) {

I expect the problem is the read of an output port is not being checked for combinational loops.

[uenoku]

Although this should be caught by CheckCombLoop but I feel hw canonicalizer should not cause assertion failures. Maybe we can add a verification that rejects trivial loops between result and operands like %0 = comb.xor %0 (it might hit the performance though).

FYI, this assertion failure comes from the following transition:

hw.module @Foo() -> (b: i1) {
  %true = hw.constant true
  %0 = comb.xor %0, %true : i1
  hw.output %0 : i1
}

==> flatten xor operands

hw.module @Foo() -> (b: i1) {
  %true = hw.constant true
  %0 = comb.xor %0, %true, %true: i1
  hw.output %0 : i1
}

==> remove %true, %true

hw.module @Foo() -> (b: i1) {
  %0 = comb.xor %0 : i1
  hw.output %0 : i1
}

[darthscsi]

andrewl@gamma28:/scratch/andrewl/circt/build$ ./bin/circt-opt --firrtl-check-comb-cycles ../loop.mlir ../loop.mlir:3:5: error: detected combinational cycle in a FIRRTL module firrtl.module @Foo(out %b: !firrtl.uint<1>) { ^ ../loop.mlir:3:28: note: Foo.b firrtl.module @Foo(out %b: !firrtl.uint<1>) { ^ ../loop.mlir:3:28: note: Foo.b

[darthscsi]

It's really weird:

andrewl@gamma28:/scratch/andrewl/circt/build$ ./bin/firtool --parse-only ../loop.fir |./bin/circt-opt --firrtl-check-comb-cycles 
<stdin>:3:5: error: detected combinational cycle in a FIRRTL module
    firrtl.module @Foo(out %b: !firrtl.uint<1>) {
    ^
<stdin>:3:28: note: Foo.b
    firrtl.module @Foo(out %b: !firrtl.uint<1>) {
                           ^
<stdin>:3:28: note: Foo.b
andrewl@gamma28:/scratch/andrewl/circt/build$ ./bin/firtool ../loop.fir 
// Generated by CIRCT unknown git version
module Foo(     // ../loop.fir:2:10
  output b);

  wire _GEN;    // ../loop.fir:5:10

  assign _GEN = ~(~_GEN);       // ../loop.fir:5:10
  assign b = ~_GEN;     // ../loop.fir:2:10, :5:10
endmodule

[darthscsi]

Really closed now: https://github.com/llvm/circt/commit/b523e45e1e4cb63ac59c5022b2deb4371cb0b806