From c84846442db7934bdbfcfaf8776bdb96fdf338cb Mon Sep 17 00:00:00 2001
From: Nandor Licker <n@ndor.email>
Date: Fri, 15 Mar 2024 20:08:03 +0200
Subject: [PATCH] [FIRParser] Do not swallow stop ops when parsing FIR (#6834)

---
 .../FIRRTL/Import/FIRParserAsserts.cpp        | 34 +++++++++----------
 test/firtool/stop.fir                         |  1 +
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/lib/Dialect/FIRRTL/Import/FIRParserAsserts.cpp b/lib/Dialect/FIRRTL/Import/FIRParserAsserts.cpp
index baaf4051d7b4..b3de1349984e 100644
--- a/lib/Dialect/FIRRTL/Import/FIRParserAsserts.cpp
+++ b/lib/Dialect/FIRRTL/Import/FIRParserAsserts.cpp
@@ -241,23 +241,6 @@ ParseResult circt::firrtl::foldWhenEncodedVerifOp(PrintFOp printOp) {
   auto opIt = std::next(printOp->getIterator());
   auto opEnd = thenBlock.end();
 
-  // optional `stop(clock, enable, ...)`
-  //
-  // FIXME: Currently, we can't detetct stopOp in the following IR:
-  //    when invCond:
-  //      printf(io.clock, UInt<1>(1), "assert: ..")
-  //      stop(io.clock, UInt<1>(1), 1)
-  // It is because `io.clock` will create another subfield op so StopOp is not
-  // the next operation. Also, we will have to modify `stopOp.clock() !=
-  // printOp.clock()` below since they are not CSEd.
-  if (opIt != opEnd) {
-    auto stopOp = dyn_cast<StopOp>(*opIt++);
-    if (!stopOp || opIt != opEnd || stopOp.getClock() != printOp.getClock() ||
-        stopOp.getCond() != printOp.getCond())
-      return success();
-    stopOp.erase();
-  }
-
   // Detect if we're dealing with a verification statement, and what flavor of
   // statement it is.
   auto fmt = printOp.getFormatString();
@@ -281,6 +264,23 @@ ParseResult circt::firrtl::foldWhenEncodedVerifOp(PrintFOp printOp) {
   else
     return success();
 
+  // optional `stop(clock, enable, ...)`
+  //
+  // FIXME: Currently, we can't detetct stopOp in the following IR:
+  //    when invCond:
+  //      printf(io.clock, UInt<1>(1), "assert: ..")
+  //      stop(io.clock, UInt<1>(1), 1)
+  // It is because `io.clock` will create another subfield op so StopOp is not
+  // the next operation. Also, we will have to modify `stopOp.clock() !=
+  // printOp.clock()` below since they are not CSEd.
+  if (opIt != opEnd) {
+    auto stopOp = dyn_cast<StopOp>(*opIt++);
+    if (!stopOp || opIt != opEnd || stopOp.getClock() != printOp.getClock() ||
+        stopOp.getCond() != printOp.getCond())
+      return success();
+    stopOp.erase();
+  }
+
   // Check if the condition of the `WhenOp` is a trivial inversion operation,
   // and remove any immediately preceding verification ops that ensure this
   // condition. This caters to the following pattern emitted by Chisel:
diff --git a/test/firtool/stop.fir b/test/firtool/stop.fir
index 1ac6d4a37cae..17c3a1b2ea4c 100644
--- a/test/firtool/stop.fir
+++ b/test/firtool/stop.fir
@@ -26,5 +26,6 @@ circuit StopAndFinishTest:
     ; CHECK:   }
     ; CHECK: }
 
+    printf(clock, cond, "Stop message")
     stop(clock, cond, 0)
     stop(clock, cond, 1)