From 71a78d61282291ae14c979d37b2d8f7224e269c9 Mon Sep 17 00:00:00 2001
From: Fabian Schuiki <fabian@schuiki.ch>
Date: Thu, 5 Oct 2023 14:55:21 -0700
Subject: [PATCH] [SV] Mark sv.xmr.ref op as pure (#6260)

Having an `sv.xmr.ref` op inside a procedural block such as
`sv.alwayscomb` triggers an assertion in `PrepareForEmission`. The pass
would identify the ref op as having side-effects and then go ahead and
try to pull it outside the procedural block. Doing so would create a
`sv.reg` op with multiple nested inout types, which breaks.

Fix the issue by marking the `sv.xmr.ref` op as pure. Taking a reference
to something does not have a side-effect. It's accessing what's behind
the reference that has side-effects.
---
 include/circt/Dialect/SV/SVInOutOps.td        |  5 ++++-
 .../ExportVerilog/prepare-for-emission.mlir   | 20 +++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/include/circt/Dialect/SV/SVInOutOps.td b/include/circt/Dialect/SV/SVInOutOps.td
index 490eac896fc7..51c36966e0db 100644
--- a/include/circt/Dialect/SV/SVInOutOps.td
+++ b/include/circt/Dialect/SV/SVInOutOps.td
@@ -110,7 +110,10 @@ def XMROp : SVOp<"xmr", []> {
   let assemblyFormat = "(`isRooted` $isRooted^)? custom<XMRPath>($path, $terminal) attr-dict `:` qualified(type($result))";
 }
 
-def XMRRefOp : SVOp<"xmr.ref", [DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
+def XMRRefOp : SVOp<"xmr.ref", [
+  DeclareOpInterfaceMethods<SymbolUserOpInterface>,
+  Pure
+]> {
   let summary = "Encode a reference to something with a hw.hierpath.";
   let description = [{
     This represents a hierarchical path, but using something which the compiler
diff --git a/test/Conversion/ExportVerilog/prepare-for-emission.mlir b/test/Conversion/ExportVerilog/prepare-for-emission.mlir
index bfa5c7d55fae..46fa41f73d72 100644
--- a/test/Conversion/ExportVerilog/prepare-for-emission.mlir
+++ b/test/Conversion/ExportVerilog/prepare-for-emission.mlir
@@ -272,3 +272,23 @@ hw.module @Issue5605(in %a: i1, in %b: i1, in %clock: i1, in %reset: i1) {
   sv.assert.concurrent posedge %clock, %reset label "assert_1" message "bar"(%1) : i2
   hw.output
 }
+
+// -----
+
+// The following use of `sv.xmr.ref` inside a procedural block would trigger an
+// assertion in `PrepareForEmission`, since the op was not marked as side-effect
+// free.
+//
+// CHECK-LABEL: hw.module @Foo
+module attributes {circt.loweringOptions = "disallowLocalVariables"} {
+  hw.module @Foo(in %a: i1) {
+    hw.wire %a sym @a : i1
+    // CHECK: sv.alwayscomb
+    sv.alwayscomb {
+      // CHECK-NEXT: sv.xmr.ref
+      %0 = sv.xmr.ref @xmr : !hw.inout<i1>
+      sv.verbatim "{{0}}" (%0) : !hw.inout<i1>
+    }
+  }
+  hw.hierpath @xmr [@Foo::@a]
+}