[FIRRTL] Improvements to XMRRefOp and XMRDerefOp (#8570)

- Adds a description to the ops
- Marks the ops as pure
- Adds the symbol user trait
- Ensures that the symbol op is a hierarchical path.

Author: rwy7

include/circt/Dialect/FIRRTL/FIRRTLExpressions.td

@@ -1582,15 +1582,28 @@ def RWProbeOp : FIRRTLOp<"ref.rwprobe",
   let assemblyFormat = "$target attr-dict `:` type($result)";
 }
 
-def XMRRefOp : FIRRTLOp<"xmr.ref"> {
+def XMRRefOp : FIRRTLOp<"xmr.ref", [
+    Pure,
+    DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
   let summary = "FIRRTL XMR operation, targetable by ref ops.";
+  let description = [{
+    Takes a (symbol-ref to a) hierarchical path and returns a reference to
+    the target. Through this reference, the target can be read or written
+    "at a distance".
+  }];
   let arguments = (ins FlatSymbolRefAttr:$ref, DefaultValuedAttr<StrAttr, "{}">:$verbatimSuffix);
   let results = (outs RefType:$dest);
   let assemblyFormat = "$ref (`,` $verbatimSuffix^)? attr-dict `:` qualified(type($dest))";
 }
 
-def XMRDerefOp : FIRRTLOp<"xmr.deref"> {
+def XMRDerefOp : FIRRTLOp<"xmr.deref", [
+    Pure,
+    DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
   let summary = "FIRRTL XMR operation, reading an XMR target.";
+  let description = [{
+    A "read at a distance", taking a (symbol-ref to a) hierarchical path,
+    returning the value of the target.
+  }];
   let arguments = (ins FlatSymbolRefAttr:$ref, DefaultValuedAttr<StrAttr, "{}">:$verbatimSuffix);
   let results = (outs PassiveType:$dest);
   let assemblyFormat = "$ref (`,` $verbatimSuffix^)? attr-dict `:` qualified(type($dest))";

lib/Dialect/FIRRTL/FIRRTLOps.cpp

@@ -6269,6 +6269,30 @@ LogicalResult RWProbeOp::verifyInnerRefs(hw::InnerRefNamespace &ns) {
   return checks(symOp.getTargetResult().getType(), symOp.getLoc());
 }
 
+LogicalResult XMRRefOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
+  auto *target = symbolTable.lookupNearestSymbolFrom(*this, getRefAttr());
+  if (!target)
+    return emitOpError("has an invalid symbol reference");
+
+  if (!isa<hw::HierPathOp>(target))
+    return emitOpError("does not target a hierpath op");
+
+  // TODO: Verify that the target's type matches the type of this op.
+  return success();
+}
+
+LogicalResult XMRDerefOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
+  auto *target = symbolTable.lookupNearestSymbolFrom(*this, getRefAttr());
+  if (!target)
+    return emitOpError("has an invalid symbol reference");
+
+  if (!isa<hw::HierPathOp>(target))
+    return emitOpError("does not target a hierpath op");
+
+  // TODO: Verify that the target's type matches the type of this op.
+  return success();
+}
+
 //===----------------------------------------------------------------------===//
 // Layer Block Operations
 //===----------------------------------------------------------------------===//

test/Dialect/FIRRTL/advanced-layer-sink.mlir

@@ -769,3 +769,23 @@ firrtl.circuit "DoNotSinkInstanceOfModuleWithPortAnno" {
     }
   }
 }
+
+// CHECK-LABEL: firrtl.circuit "SinkXMRs"
+firrtl.circuit "SinkXMRs" {
+  firrtl.layer @A bind {}
+
+  hw.hierpath @xmr [@SinkXMRs::@target]
+
+  firrtl.module public @SinkXMRs() {
+    %target = firrtl.wire sym @target : !firrtl.uint<1>
+    %0 = firrtl.xmr.deref @xmr : !firrtl.uint<1>
+    %1 = firrtl.xmr.ref @xmr : !firrtl.ref<uint<1>>
+  
+    // CHECK: firrtl.layerblock @A
+    firrtl.layerblock @A {
+      // CHECK-NEXT: %0 = firrtl.xmr.deref @xmr : !firrtl.uint<1>
+      // CHECK-NEXT: %1 = firrtl.xmr.ref @xmr : !firrtl.probe<uint<1>>
+      "unknown"(%0, %1) : (!firrtl.uint<1>, !firrtl.ref<uint<1>>) -> ()
+    }
+  }
+}

test/Dialect/FIRRTL/canonicalization.mlir

@@ -3635,4 +3635,17 @@ firrtl.module @name_prop(in %clock: !firrtl.clock, in %next: !firrtl.uint<8>, ou
   %m = firrtl.node %wire : !firrtl.uint<8>
   firrtl.connect %out_b, %m : !firrtl.uint<8>, !firrtl.uint<8>
 }
+
+hw.hierpath @xmr [@XMRTest::@target]
+
+// CHECK-LABEL: firrtl.module private @XMRTest
+firrtl.module private @XMRTest() {
+  %target = firrtl.wire sym @target : !firrtl.uint<1>
+
+  // CHECK-NOT: firrtl.xmr.deref
+  %0 = firrtl.xmr.deref @xmr : !firrtl.uint<1>
+
+  // CHECK-NOT: firrtl.xmr.ref
+  %1 = firrtl.xmr.ref @xmr : !firrtl.ref<uint<1>>
+}
 }

test/Dialect/FIRRTL/errors.mlir

@@ -2925,3 +2925,41 @@ firrtl.circuit "BindTargetMissingDoNotPrintFlag" {
   // expected-error @below {{target #hw.innerNameRef<@BindTargetMissingDoNotPrintFlag::@target> is not marked doNotPrint}}
   firrtl.bind <@BindTargetMissingDoNotPrintFlag::@target>
 }
+
+// -----
+
+firrtl.circuit "XMRRefOpMissingTarget" {
+  firrtl.module @XMRRefOpMissingTarget() {
+    // expected-error @below {{op has an invalid symbol reference}}
+    %0 = firrtl.xmr.ref @MissingTarget : !firrtl.ref<uint<1>>
+  }
+}
+
+// -----
+
+firrtl.circuit "XMRRefOpTargetsNonHierPath" {
+  firrtl.extmodule @Target()
+  firrtl.module @XMRRefOpMissingTarget() {
+    // expected-error @below {{op does not target a hierpath op}}
+    %0 = firrtl.xmr.ref @Target : !firrtl.ref<uint<1>>
+  }
+}
+
+// -----
+
+firrtl.circuit "XMRDerefOpMissingTarget" {
+  firrtl.module @XMRDerefOpMissingTarget() {
+    // expected-error @below {{op has an invalid symbol reference}}
+    %0 = firrtl.xmr.deref @MissingTarget : !firrtl.uint<1>
+  }
+}
+
+// -----
+
+firrtl.circuit "XMRDerefOpTargetsNonHierPath" {
+  firrtl.extmodule @Target()
+  firrtl.module @XMRDerefOpTargetsNonHierPath() {
+    // expected-error @below {{op does not target a hierpath op}}
+    %0 = firrtl.xmr.deref @Target : !firrtl.uint<1>
+  }
+}

test/Dialect/FIRRTL/lower-layers.mlir

@@ -138,6 +138,7 @@ firrtl.circuit "Test" {
   // CHECK-NEXT:   %w_probe = firrtl.node sym @sym interesting_name %w : !firrtl.uint<1>
   // CHECK-NEXT:   firrtl.instance {{.+}} {doNotPrint, output_file = #hw.output_file<"layers-CaptureProbeSrc-A.sv", excludeFromFileList>} @CaptureProbeSrc_A
   // CHECK-NEXT: }
+  hw.hierpath private @xmrPath [@CaptureProbeSrc::@sym]
   firrtl.module @CaptureProbeSrc() {
     %w = firrtl.wire : !firrtl.uint<1>
     %w_probe = firrtl.node sym @sym interesting_name %w : !firrtl.uint<1>
@@ -310,13 +311,13 @@ firrtl.circuit "Test" {
   // XMR Ref ops used by force_initial are cloned.
   //
   // CHECK:      firrtl.module private @XmrRef_A()
-  // CHECK-NEXT:   %0 = firrtl.xmr.ref @RefXmrRef_path : !firrtl.rwprobe<uint<1>, @A>
+  // CHECK-NEXT:   %0 = firrtl.xmr.ref @XmrRef_path : !firrtl.rwprobe<uint<1>, @A>
   // CHECK-NEXT:   %a = firrtl.wire
   // CHECK-NEXT:   %c1_ui1 = firrtl.constant 1
   // CHECK-NEXT:   firrtl.ref.force_initial %c1_ui1, %0, %c1_ui1
   hw.hierpath private @XmrRef_path [@XmrRef::@a]
   firrtl.module @XmrRef() {
-    %0 = firrtl.xmr.ref @RefXmrRef_path : !firrtl.rwprobe<uint<1>, @A>
+    %0 = firrtl.xmr.ref @XmrRef_path : !firrtl.rwprobe<uint<1>, @A>
     firrtl.layerblock @A {
       %a = firrtl.wire sym @a : !firrtl.uint<1>
       %c1_ui1 = firrtl.constant 1 : !firrtl.const.uint<1>

test/firtool/sink-probes-into-layers.fir

@@ -0,0 +1,46 @@
+; RUN: firtool --disable-all-randomization --advanced-layer-sink %s | FileCheck %s
+
+; Check that the probes are correctly sunk into the layer, and there are
+; no bare probes left in the design.
+
+; Check the probes are inlined into the assertions:
+
+; CHECK-LABEL: module Foo_A();
+; CHECK-NEXT:    always @(posedge Foo.clock) begin
+; CHECK-NEXT:      assert(Foo.bar.`ref_Bar_probe) else $error("message");
+; CHECK-NEXT:      assert(Foo.bar.`ref_Bar_probe) else $error("message");
+; CHECK-NEXT:    end // always @(posedge)
+; CHECK-NEXT:    `ifndef SYNTHESIS
+; CHECK-NEXT:       initial
+; CHECK-NEXT:         force Foo.bar.`ref_Bar_rwprobe = 1'h1;
+; CHECK-NEXT:    `endif // not def SYNTHESIS
+; CHECK-NEXT:  endmodule
+
+; Check that there are no probes in the top level module:
+
+; CHECK-LABEL: module Foo(
+; CHECK-NEXT:    input clock
+; CHECK-NEXT:  );
+; CHECK-EMPTY:
+; CHECK-NEXT:    Bar bar ();
+; CHECK-NEXT:  endmodule
+
+FIRRTL version 4.0.0
+
+circuit Foo:
+  layer A, bind:
+
+  public module Foo:
+    input clock: Clock
+
+    inst bar of Bar
+    node cond = read(bar.probe)
+
+    layerblock A:
+      assert(clock, cond, UInt<1>(1), "message")
+      assert(clock, cond, UInt<1>(1), "message")
+      force_initial(bar.rwprobe, UInt<1>(1))
+
+  extmodule Bar:
+    output probe : Probe<UInt<1>>
+    output rwprobe : RWProbe<UInt<1>>