[FIRRTL] Squash Invalids in RemoveInvalid

Extend the RemoveInvalid pass (formerly RemoveResets) to convert all
invalid values to zero.  This is done to apply the "invalid is zero"
interpretation after all other invalid interpretations have been
resolved.  This replaces logic where this interpretation was
incompletely applied during canonicalization after RemoveResets.

Fixes #2782.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
This commit is contained in:
Schuyler Eldridge 2022-04-09 17:54:20 -04:00 committed by Schuyler Eldridge
parent 988af4791f
commit cb5d02799c
3 changed files with 70 additions and 3 deletions

View File

@ -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<RegResetOp>())) {
SmallVector<InvalidValueOp> invalidOps;
for (auto &op : llvm::make_early_inc_range(getOperation().getOps())) {
// Populate invalidOps for later handling.
if (auto inv = dyn_cast<InvalidValueOp>(op)) {
invalidOps.push_back(inv);
continue;
}
auto reg = dyn_cast<RegResetOp>(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<FIRRTLType>(inv.getType())
.Case<ClockType, AsyncResetType, ResetType>([&](auto type) {
auto zero = builder.create<SpecialConstantOp>(
type, builder.getBoolAttr(false));
inv->replaceAllUsesWith(zero);
inv->erase();
madeModifications = true;
})
.Case<IntType>([&](IntType type) {
auto zero = builder.create<ConstantOp>(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();
}

View File

@ -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

View File

@ -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: }