diff --git a/lib/Dialect/FIRRTL/Transforms/RemoveInvalid.cpp b/lib/Dialect/FIRRTL/Transforms/RemoveInvalid.cpp index 4a9fcd7479..de5525d75d 100644 --- a/lib/Dialect/FIRRTL/Transforms/RemoveInvalid.cpp +++ b/lib/Dialect/FIRRTL/Transforms/RemoveInvalid.cpp @@ -15,6 +15,7 @@ //===----------------------------------------------------------------------===// #include "PassDetails.h" +#include "circt/Dialect/FIRRTL/FIRRTLUtils.h" #include "circt/Dialect/FIRRTL/Passes.h" #include "mlir/IR/ImplicitLocOpBuilder.h" #include "llvm/ADT/TypeSwitch.h" @@ -100,8 +101,16 @@ void RemoveInvalidPass::runOnOperation() { << "Module: '" << getOperation().getName() << "'\n";); bool madeModifications = false; - for (auto reg : llvm::make_early_inc_range( - getOperation().getBody()->getOps())) { + SmallVector invalidOps; + for (auto &op : llvm::make_early_inc_range(getOperation().getOps())) { + // Populate invalidOps for later handling. + if (auto inv = dyn_cast(op)) { + invalidOps.push_back(inv); + continue; + } + auto reg = dyn_cast(op); + if (!reg) + continue; // If the `RegResetOp` has an invalidated initialization, then replace it // with a `RegOp`. @@ -118,6 +127,33 @@ void RemoveInvalidPass::runOnOperation() { } } + // Convert all invalid values to zero. + for (auto inv : invalidOps) { + // Skip invalids which have no uses. + if (inv->getUses().empty()) + continue; + ImplicitLocOpBuilder builder(inv.getLoc(), inv); + TypeSwitch(inv.getType()) + .Case([&](auto type) { + auto zero = builder.create( + type, builder.getBoolAttr(false)); + inv->replaceAllUsesWith(zero); + inv->erase(); + madeModifications = true; + }) + .Case([&](IntType type) { + auto zero = builder.create(type, getIntZerosAttr(type)); + inv->replaceAllUsesWith(zero); + inv->erase(); + madeModifications = true; + }) + .Default([&](auto) { + inv->emitOpError() + << "has a type which is not a ground type (this is an " + "unimplemented feature of RemoveInvalid!)"; + }); + } + if (!madeModifications) return markAllAnalysesPreserved(); } diff --git a/test/Dialect/FIRRTL/SFCTests/invalid-interpretations.fir b/test/Dialect/FIRRTL/SFCTests/invalid-interpretations.fir index 893768cae1..75e91121b4 100644 --- a/test/Dialect/FIRRTL/SFCTests/invalid-interpretations.fir +++ b/test/Dialect/FIRRTL/SFCTests/invalid-interpretations.fir @@ -42,3 +42,34 @@ circuit InvalidInterpretations: ; Interpretation 4: Invalid is zero otherwise. ; CHECK: assign out_mux = cond ? a : 8'h0; ; CHECK-NEXT: assign out_add = {1'h0, a}; + +; // ----- + +; This is checking that an invalid value in another module is not propagated, +; but is interpreted as "zero". The end result of this is that the main module +; should have a register that will be reset to a non-zero value, but is +; constantly driven to zero by default. +; +; See: https://github.com/llvm/circt/issues/2782 + +; CHECK-LABEL: module InvalidInOtherModule +circuit InvalidInOtherModule : + module InvalidInOtherModule : + input clock: Clock + input reset: UInt<1> + output b: SInt<8> + + inst other of OtherModule + + ; CHECK: always @(posedge clock) + ; CHECK-NEXT: if (reset) + ; CHECK-NEXT: r <= 8'h4; + ; CHECK-NEXT: else + ; CHECK-NEXT: r <= 8'h0; + reg r : SInt<8>, clock with: (reset => (reset, SInt<8>(4))) + r <= other.b + b <= r + + module OtherModule : + output b: SInt<8> + b is invalid diff --git a/test/firtool/firtool.fir b/test/firtool/firtool.fir index 66720ab066..f5d01890c8 100644 --- a/test/firtool/firtool.fir +++ b/test/firtool/firtool.fir @@ -105,7 +105,7 @@ circuit test_mod : %[[{"a": "a"}]] ; MLIR-NEXT: firrtl.strictconnect %multibitMux_a_1, %vec_1 : !firrtl.uint<1> ; MLIR-NEXT: firrtl.strictconnect %multibitMux_a_2, %vec_2 : !firrtl.uint<1> ; MLIR-NEXT: firrtl.strictconnect %multibitMux_sel, %b : !firrtl.uint<2> -; MLIR-NEXT: %invalid_ui1 = firrtl.invalidvalue : !firrtl.uint<1> +; MLIR-NEXT: %c0_ui1 = firrtl.constant 0 : !firrtl.uint<1> ; MLIR-NEXT: firrtl.instance unusedPortsMod @UnusedPortsMod() ; MLIR-NEXT: }