Skip to content

Commit f291e58

Browse files
youngarjackkoenig
authored andcommittedOct 8, 2024
[FIRRTL] InferResets: properly lower FART'd registers
This is quick fix for #7675. The way this is supposed to work is, FRT is not supposed to add a reset if a register already has one, but it isn't smart enough to understand that an invalid value corresponds to a register without a reset. As a result, it will not add a reset to this register. Later on, in SFCCompat, when choosing how to lower the Invalid value, it should "see" that the registers should all have a reset, by looking for the FRT annotation, and choose to lower the reset value to 0 or remove the reset accordingly. The "bug" is that the FRT annotation on a parent module won't be discovered on the child module. This attaches the annotation on to every module in a reset domain, so that it can be found.
1 parent 804cdbe commit f291e58

File tree

2 files changed

+35
-0
lines changed

2 files changed

+35
-0
lines changed
 

‎lib/Dialect/FIRRTL/Transforms/InferResets.cpp

+8
Original file line numberDiff line numberDiff line change
@@ -1685,6 +1685,14 @@ LogicalResult InferResetsPass::implementFullReset(FModuleOp module,
16851685
return success();
16861686
}
16871687

1688+
// Add an annotation indicating that this module belongs to a reset domain.
1689+
auto *context = module.getContext();
1690+
AnnotationSet annotations(module);
1691+
annotations.addAnnotations(DictionaryAttr::get(
1692+
context, NamedAttribute(StringAttr::get(context, "class"),
1693+
StringAttr::get(context, fullResetAnnoClass))));
1694+
annotations.applyToOperation(module);
1695+
16881696
// If needed, add a reset port to the module.
16891697
Value actualReset = domain.existingValue;
16901698
if (domain.newPortName) {

‎test/Dialect/FIRRTL/infer-resets.mlir

+27
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,33 @@ firrtl.circuit "ZeroWidthRegister" {
845845

846846
// -----
847847

848+
// Every module which is contained inside a reset domain should be annotated as
849+
// such, so that internal registers can be lowered later correctly.
850+
// https://github.com/llvm/circt/issues/7675
851+
firrtl.circuit "top" {
852+
// CHECK: firrtl.module private @test
853+
// CHECK-SAME: annotations = [{class = "circt.FullResetAnnotation"}]
854+
firrtl.module private @test(in %clock: !firrtl.clock, in %reset: !firrtl.asyncreset, in %in: !firrtl.uint<8>, out %out: !firrtl.uint<8>) {
855+
%resetvalue = firrtl.wire : !firrtl.uint<8>
856+
%invalid_ui8 = firrtl.invalidvalue : !firrtl.uint<8>
857+
firrtl.matchingconnect %resetvalue, %invalid_ui8 : !firrtl.uint<8>
858+
%reg1 = firrtl.regreset %clock, %reset, %resetvalue : !firrtl.clock, !firrtl.asyncreset, !firrtl.uint<8>, !firrtl.uint<8>
859+
firrtl.matchingconnect %reg1, %in : !firrtl.uint<8>
860+
firrtl.matchingconnect %out, %reg1 : !firrtl.uint<8>
861+
}
862+
// CHECK: firrtl.module @top
863+
// CHECK-SAME: annotations = [{class = "circt.FullResetAnnotation"}]
864+
firrtl.module @top(in %clock: !firrtl.clock, in %reset: !firrtl.asyncreset [{class = "circt.FullResetAnnotation", resetType = "async"}], in %in: !firrtl.uint<8>, out %out: !firrtl.uint<8>) attributes {convention = #firrtl<convention scalarized>} {
865+
%child_clock, %child_reset, %child_in, %child_out = firrtl.instance child @test(in clock: !firrtl.clock, in reset: !firrtl.asyncreset, in in: !firrtl.uint<8>, out out: !firrtl.uint<8>)
866+
firrtl.matchingconnect %child_clock, %clock : !firrtl.clock
867+
firrtl.matchingconnect %child_reset, %reset : !firrtl.asyncreset
868+
firrtl.matchingconnect %child_in, %in : !firrtl.uint<8>
869+
firrtl.matchingconnect %out, %child_out : !firrtl.uint<8>
870+
}
871+
}
872+
873+
// -----
874+
848875
// Check that unaffected fields ("data") are not being affected by width
849876
// inference. See https://github.com/llvm/circt/issues/2857.
850877
// CHECK-LABEL: firrtl.module @ZeroLengthVectorInBundle1

0 commit comments

Comments
 (0)
Please sign in to comment.