Skip to content

Commit 52e3074

Browse files
[FIRRTL] Fix dedup looking up wrong op in inner ref target check (#9104)
Fix an issue in the Dedup pass, where the pass would use `data.map.lookup(...)` to resolve the target operation of an inner reference from one module to another, but it would use op B as lookup key, while the map contains mappings from A to B.
1 parent 8e02668 commit 52e3074

File tree

2 files changed

+26
-1
lines changed

2 files changed

+26
-1
lines changed

lib/Dialect/FIRRTL/Transforms/Dedup.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ struct Equivalence {
717717
} else {
718718
// Otherwise make sure that they are targeting the same operation.
719719
if (!bTarget.isOpOnly() ||
720-
aTarget.getOp() != data.map.lookup(bTarget.getOp()))
720+
data.map.lookupOrNull(aTarget.getOp()) != bTarget.getOp())
721721
return error();
722722
}
723723
if (aTarget.getField() != bTarget.getField())

test/Dialect/FIRRTL/dedup-errors.mlir

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,3 +681,28 @@ firrtl.circuit "VisibilityDoesNotBlockDedup" attributes {annotations = [{
681681
firrtl.instance test1 @Test1()
682682
}
683683
}
684+
685+
// -----
686+
// When reporting on a dedup failure, use `data.map.lookup()` on the correct
687+
// copy of the target operation. This used to trigger an assertion failure when
688+
// comparing the rwprobes even though the dedup failure was a trivial mismatch
689+
// in the number of operations.
690+
// See https://github.com/llvm/circt/issues/9102
691+
692+
// expected-error @below {{module "Bar" not deduplicated with "Foo"}}
693+
firrtl.circuit "Foo" attributes {annotations = [{
694+
class = "firrtl.transforms.MustDeduplicateAnnotation",
695+
modules = ["~Foo|Foo", "~Foo|Bar"]
696+
}]} {
697+
// expected-note @below {{first block here}}
698+
firrtl.module @Foo() {
699+
%0 = firrtl.wire sym @sym : !firrtl.uint<42>
700+
%1 = firrtl.ref.rwprobe <@Foo::@sym> : !firrtl.rwprobe<uint<42>>
701+
}
702+
firrtl.module private @Bar() {
703+
%0 = firrtl.wire sym @sym : !firrtl.uint<42>
704+
%1 = firrtl.ref.rwprobe <@Bar::@sym> : !firrtl.rwprobe<uint<42>>
705+
// expected-note @below {{second block has more operations}}
706+
%2 = firrtl.wire : !firrtl.uint<42>
707+
}
708+
}

0 commit comments

Comments
 (0)