Update annotation handling to fix partial field reset behavior (llvm#…

…6912) Update annotation handling in InferResets and SFCCompat
cepheus69 · Apr 22, 2024 · bb1cb2f · bb1cb2f
1 parent ae819a7
commit bb1cb2f
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 17 deletions.
diff --git a/lib/Dialect/FIRRTL/Transforms/InferResets.cpp b/lib/Dialect/FIRRTL/Transforms/InferResets.cpp
@@ -1308,7 +1308,7 @@ InferResetsPass::collectAnnos(FModuleOp module) {
       reset = arg;
       conflictingAnnos.insert({anno, reset.getLoc()});
 
-      return true;
+      return false;
     }
     if (anno.isClass(ignoreFullAsyncResetAnnoClass)) {
       anyFailed = true;
@@ -1350,7 +1350,7 @@ InferResetsPass::collectAnnos(FModuleOp module) {
         }
         reset = op->getResult(0);
         conflictingAnnos.insert({anno, reset.getLoc()});
-        return true;
+        return false;
       }
       if (anno.isClass(ignoreFullAsyncResetAnnoClass)) {
         anyFailed = true;

diff --git a/lib/Dialect/FIRRTL/Transforms/SFCCompat.cpp b/lib/Dialect/FIRRTL/Transforms/SFCCompat.cpp
@@ -19,6 +19,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "PassDetails.h"
+#include "circt/Dialect/FIRRTL/AnnotationDetails.h"
 #include "circt/Dialect/FIRRTL/FIRRTLOps.h"
 #include "circt/Dialect/FIRRTL/FIRRTLUtils.h"
 #include "circt/Dialect/FIRRTL/Passes.h"
@@ -44,6 +45,15 @@ void SFCCompatPass::runOnOperation() {
 
   bool madeModifications = false;
   SmallVector<InvalidValueOp> invalidOps;
+
+  bool fullAsyncResetExists = false;
+  AnnotationSet::removePortAnnotations(
+      getOperation(), [&](unsigned argNum, Annotation anno) {
+        if (!anno.isClass(fullAsyncResetAnnoClass))
+          return false;
+        return fullAsyncResetExists = true;
+      });
+
   auto result = getOperation()->walk([&](Operation *op) {
     // Populate invalidOps for later handling.
     if (auto inv = dyn_cast<InvalidValueOp>(op)) {
@@ -54,9 +64,10 @@ void SFCCompatPass::runOnOperation() {
     if (!reg)
       return WalkResult::advance();
 
-    // If the `RegResetOp` has an invalidated initialization, then replace it
-    // with a `RegOp`.
-    if (walkDrivers(reg.getResetValue(), true, false, false,
+    // 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,
                     [](FieldRef dst, FieldRef src) {
                       return src.isa<InvalidValueOp>();
                     })) {

diff --git a/test/Dialect/FIRRTL/infer-resets.mlir b/test/Dialect/FIRRTL/infer-resets.mlir
@@ -362,18 +362,6 @@ firrtl.module @ForeignTypes(out %out: !firrtl.reset) {
 firrtl.module @ConsumeIgnoreAnno() attributes {annotations = [{class = "sifive.enterprise.firrtl.IgnoreFullAsyncResetAnnotation"}]} {
 }
 
-// CHECK-LABEL: firrtl.module @ConsumeResetAnnoPort
-// CHECK-NOT: FullAsyncResetAnnotation
-firrtl.module @ConsumeResetAnnoPort(in %outerReset: !firrtl.asyncreset) attributes {portAnnotations = [[{class = "sifive.enterprise.firrtl.FullAsyncResetAnnotation"}]]} {
-}
-
-// CHECK-LABEL: firrtl.module @ConsumeResetAnnoWire
-firrtl.module @ConsumeResetAnnoWire(in %outerReset: !firrtl.asyncreset) {
-  // CHECK: %innerReset = firrtl.wire
-  // CHECK-NOT: FullAsyncResetAnnotation
-  %innerReset = firrtl.wire {annotations = [{class = "sifive.enterprise.firrtl.FullAsyncResetAnnotation"}]} : !firrtl.asyncreset
-}
-
 } // firrtl.circuit
 
 // -----

diff --git a/test/firtool/async-reset-anno.fir b/test/firtool/async-reset-anno.fir
@@ -0,0 +1,19 @@
+; RUN: firtool %s -parse-only | circt-opt -firrtl-infer-resets | FileCheck %s --check-prefix POST-INFER-RESETS
+; RUN: firtool %s -parse-only | circt-opt -firrtl-infer-resets -firrtl-sfc-compat | FileCheck %s --implicit-check-not POST-SFC-COMPAT-NOT
+
+; Check that FullAsyncResetAnnotation exists after infer-resets pass
+; but is deleted after sfc-compat
+
+FIRRTL version 3.3.0
+circuit test :%[[{
+  "class":"sifive.enterprise.firrtl.FullAsyncResetAnnotation",
+    "target":"~test|test>reset"
+    }]]
+  module test :
+    input clock : Clock
+    input reset : AsyncReset
+    ; POST-INFER-RESETS: [{class = "sifive.enterprise.firrtl.FullAsyncResetAnnotation"}]
+    ; POST-SFC-COMPAT-NOT: [{class = "sifive.enterprise.firrtl.FullAsyncResetAnnotation"}]
+    input in : { foo : UInt<8>, bar : UInt<8>}
+    output out : { foo : UInt<8>, bar : UInt<8>}
+    connect out, in
diff --git a/test/firtool/async-reset.fir b/test/firtool/async-reset.fir
@@ -0,0 +1,27 @@
+; RUN: firtool %s | FileCheck %s
+
+FIRRTL version 3.3.0
+circuit test :%[[{
+  "class":"sifive.enterprise.firrtl.FullAsyncResetAnnotation",
+    "target":"~test|test>reset"
+  }]]
+  module test :
+    input clock : Clock
+    input reset : AsyncReset
+    input in : { foo : UInt<8>, bar : UInt<8>}
+    output out : { foo : UInt<8>, bar : UInt<8>}
+
+    wire reg1_w : { foo : UInt<8>, bar : UInt<8>}
+    invalidate reg1_w.bar
+    invalidate reg1_w.foo
+    ; CHECK: reg1_foo <= 8'hC;
+    ; CHECK: reg1_bar <= 8'h0;
+    connect reg1_w.foo, UInt<8>(0hc)
+    invalidate reg1_w.bar
+    ; CHECK: reg1_foo = 8'hC;
+    ; CHECK: reg1_bar = 8'h0;
+    regreset reg1 : { foo : UInt<8>, bar : UInt<8>}, clock, reset, reg1_w
+    wire reg2 : { foo : UInt<8>, bar : UInt<8>}
+    connect reg1, in
+    connect reg2, reg1
+    connect out, reg2