From da2ca8cc9f8a305a0bcc3d3ac61175402138babb Mon Sep 17 00:00:00 2001 From: Andrew Lenharth Date: Tue, 21 May 2024 22:57:18 -0500 Subject: [PATCH] [FIRRTL] Register reset elimination based on invalid can look through nodes. (#7069) This converts wires into nodes when there is one write to the wire and it dominates the reads. By converting to nodes, this pass does not have to worry about symbols, references, or annotations. Those are just copied to the node. --- lib/Dialect/FIRRTL/Transforms/InferResets.cpp | 2 +- lib/Dialect/FIRRTL/Transforms/SFCCompat.cpp | 2 +- test/Dialect/FIRRTL/SFCTests/data-taps.fir | 2 ++ test/Dialect/FIRRTL/SFCTests/dedup.fir | 2 +- test/Dialect/FIRRTL/sfc-compat.mlir | 5 +++-- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/InferResets.cpp b/lib/Dialect/FIRRTL/Transforms/InferResets.cpp index 951813353dfb..7c4299183abc 100644 --- a/lib/Dialect/FIRRTL/Transforms/InferResets.cpp +++ b/lib/Dialect/FIRRTL/Transforms/InferResets.cpp @@ -665,7 +665,7 @@ static bool getDeclName(Value value, SmallString<32> &string) { op.getPortName(cast(value).getResultNumber()).getValue(); return true; }) - .Case([&](auto op) { + .Case([&](auto op) { string += op.getName(); return true; }) diff --git a/lib/Dialect/FIRRTL/Transforms/SFCCompat.cpp b/lib/Dialect/FIRRTL/Transforms/SFCCompat.cpp index e8bc2b20ab2f..80fa52b31cb9 100644 --- a/lib/Dialect/FIRRTL/Transforms/SFCCompat.cpp +++ b/lib/Dialect/FIRRTL/Transforms/SFCCompat.cpp @@ -75,7 +75,7 @@ void SFCCompatPass::runOnOperation() { // If the `RegResetOp` has an invalidated initialization and we // are not running FART, then replace it with a `RegOp`. if (!fullAsyncResetExists && - walkDrivers(reg.getResetValue(), true, false, false, + walkDrivers(reg.getResetValue(), true, true, false, [](FieldRef dst, FieldRef src) { return src.isa(); })) { diff --git a/test/Dialect/FIRRTL/SFCTests/data-taps.fir b/test/Dialect/FIRRTL/SFCTests/data-taps.fir index 4c1171a3aba7..535a9b73be4f 100644 --- a/test/Dialect/FIRRTL/SFCTests/data-taps.fir +++ b/test/Dialect/FIRRTL/SFCTests/data-taps.fir @@ -200,6 +200,8 @@ circuit Top : %[[ io.d <= d ; CHECK: module Top + ; TODO: fix having constants carry names + ; CHECK: wire inv = 1'h0 ; CHECK: io_b = Top.foo.f_probe; ; CHECK-NEXT: io_c = Top.foo.g_probe; ; CHECK-NEXT: io_d = inv; diff --git a/test/Dialect/FIRRTL/SFCTests/dedup.fir b/test/Dialect/FIRRTL/SFCTests/dedup.fir index cc0368676c58..e3f6bd2dcc22 100644 --- a/test/Dialect/FIRRTL/SFCTests/dedup.fir +++ b/test/Dialect/FIRRTL/SFCTests/dedup.fir @@ -905,7 +905,7 @@ circuit Top18 : %[[ ; CHECK: module private @A( module A : output x: UInt<1> - ; CHECK-NEXT: firrtl.wire + ; CHECK: firrtl.wire ; CHECK-SAME: [{class = "firrtl.transforms.DontTouchAnnotation"}] wire b: UInt<1> b is invalid diff --git a/test/Dialect/FIRRTL/sfc-compat.mlir b/test/Dialect/FIRRTL/sfc-compat.mlir index 5613a140d963..92e0fefabcee 100644 --- a/test/Dialect/FIRRTL/sfc-compat.mlir +++ b/test/Dialect/FIRRTL/sfc-compat.mlir @@ -93,13 +93,14 @@ firrtl.circuit "SFCCompatTests" { firrtl.connect %q, %r : !firrtl.uint<1>, !firrtl.uint<1> } - // A regreset invalid value should NOT propagate through a node. + // A regreset invalid value should propagate through a node. + // Change from SFC behavior. firrtl.module @InvalidNode(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>, in %d: !firrtl.uint<8>, out %q: !firrtl.uint<8>) { %inv = firrtl.wire : !firrtl.uint<8> %invalid_ui8 = firrtl.invalidvalue : !firrtl.uint<8> firrtl.connect %inv, %invalid_ui8 : !firrtl.uint<8>, !firrtl.uint<8> %_T = firrtl.node %inv : !firrtl.uint<8> - // CHECK: firrtl.regreset %clock + // CHECK: firrtl.reg %clock %r = firrtl.regreset %clock, %reset, %_T : !firrtl.clock, !firrtl.uint<1>, !firrtl.uint<8>, !firrtl.uint<8> firrtl.connect %r, %d : !firrtl.uint<8>, !firrtl.uint<8> firrtl.connect %q, %r : !firrtl.uint<8>, !firrtl.uint<8>