[Arc] Add option to observe registers and memories (#6477)

Instead of always making registers and memories observable for the user,
add dedicated `--observe-registers` and `--observe-memories` options to
`arcilator`. These allow the user to indicate whether they are
interested in observing or register and memory state. If these options
are set to false, don't attach "name" or "names" attributes to states
and memories, such that other parts of the pipeline have a chance to
delete unnecessary state.

Author: fabianschuiki

include/circt/Conversion/ConvertToArcs.h

@@ -13,7 +13,13 @@
 #include <memory>
 
 namespace circt {
-std::unique_ptr<OperationPass<ModuleOp>> createConvertToArcsPass();
+
+#define GEN_PASS_DECL_CONVERTTOARCS
+#include "circt/Conversion/Passes.h.inc"
+
+std::unique_ptr<OperationPass<ModuleOp>>
+createConvertToArcsPass(const ConvertToArcsOptions &options = {});
+
 } // namespace circt
 
 #endif // CIRCT_CONVERSION_CONVERTTOARCS_H

include/circt/Conversion/Passes.td

@@ -614,6 +614,10 @@ def ConvertToArcs : Pass<"convert-to-arcs", "mlir::ModuleOp"> {
   }];
   let constructor = "circt::createConvertToArcsPass()";
   let dependentDialects = ["circt::arc::ArcDialect"];
+  let options = [
+    Option<"tapRegisters", "tap-registers", "bool", "true",
+      "Make registers observable">,
+  ];
 }
 
 //===----------------------------------------------------------------------===//

include/circt/Dialect/Arc/ArcPasses.td

@@ -74,6 +74,8 @@ def InferMemories : Pass<"arc-infer-memories", "mlir::ModuleOp"> {
   let options = [
     Option<"tapPorts", "tap-ports", "bool", "true",
       "Make memory ports observable">,
+    Option<"tapMemories", "tap-memories", "bool", "true",
+      "Make memory contents observable">,
   ];
 }
 

lib/Conversion/ConvertToArcs/ConvertToArcs.cpp

@@ -59,6 +59,10 @@ struct Converter {
 
   /// The arc uses generated by `extractArcs`.
   SmallVector<StateOp> arcUses;
+
+  /// Whether registers should be made observable by assigning their arcs a
+  /// "name" attribute.
+  bool tapRegisters;
 };
 } // namespace
 
@@ -327,7 +331,7 @@ LogicalResult Converter::absorbRegs(HWModuleOp module) {
             "had a reset.");
       arc.getResetMutable().assign(reset);
     }
-    if (llvm::any_of(absorbedNames, [](auto name) {
+    if (tapRegisters && llvm::any_of(absorbedNames, [](auto name) {
           return !name.template cast<StringAttr>().getValue().empty();
         }))
       arc->setAttr("names", ArrayAttr::get(module.getContext(), absorbedNames));
@@ -399,7 +403,7 @@ LogicalResult Converter::absorbRegs(HWModuleOp module) {
     auto reset = std::get<1>(clockAndResetAndOp);
     if (reset)
       arcOp.getResetMutable().assign(reset);
-    if (llvm::any_of(names, [](auto name) {
+    if (tapRegisters && llvm::any_of(names, [](auto name) {
           return !name.template cast<StringAttr>().getValue().empty();
         }))
       arcOp->setAttr("names", builder.getArrayAttr(names));
@@ -420,16 +424,25 @@ LogicalResult Converter::absorbRegs(HWModuleOp module) {
 // Pass Infrastructure
 //===----------------------------------------------------------------------===//
 
+namespace circt {
+#define GEN_PASS_DEF_CONVERTTOARCS
+#include "circt/Conversion/Passes.h.inc"
+} // namespace circt
+
 namespace {
-struct ConvertToArcsPass : public ConvertToArcsBase<ConvertToArcsPass> {
+struct ConvertToArcsPass : public impl::ConvertToArcsBase<ConvertToArcsPass> {
+  using ConvertToArcsBase::ConvertToArcsBase;
+
   void runOnOperation() override {
     Converter converter;
+    converter.tapRegisters = tapRegisters;
     if (failed(converter.run(getOperation())))
       signalPassFailure();
   }
 };
 } // namespace
 
-std::unique_ptr<OperationPass<ModuleOp>> circt::createConvertToArcsPass() {
-  return std::make_unique<ConvertToArcsPass>();
+std::unique_ptr<OperationPass<ModuleOp>>
+circt::createConvertToArcsPass(const ConvertToArcsOptions &options) {
+  return std::make_unique<ConvertToArcsPass>(options);
 }

lib/Dialect/Arc/Transforms/InferMemories.cpp

@@ -116,7 +116,7 @@ void InferMemoriesPass::runOnOperation() {
     }
     auto memType = MemoryType::get(&getContext(), depth, wordType, addressTy);
     auto memOp = builder.create<MemoryOp>(memType);
-    if (!instOp.getInstanceName().empty())
+    if (tapMemories && !instOp.getInstanceName().empty())
       memOp->setAttr("name", instOp.getInstanceNameAttr());
 
     unsigned argIdx = 0;

test/Conversion/ConvertToArcs/convert-to-arcs-names.mlir

@@ -0,0 +1,11 @@
+// RUN: circt-opt %s --convert-to-arcs="tap-registers=0" | FileCheck %s --check-prefixes=CHECK,CHECK-TAP-OFF
+// RUN: circt-opt %s --convert-to-arcs="tap-registers=1" | FileCheck %s --check-prefixes=CHECK,CHECK-TAP-ON
+
+// CHECK-LABEL: hw.module @Trivial(
+hw.module @Trivial(in %clock: !seq.clock, in %i0: i4, in %reset: i1, out o0: i4) {
+  // CHECK: arc.state {{@.+}}(%i0) clock %clock lat 1
+  // CHECK-TAP-OFF-NOT: names = ["foo"]
+  // CHECK-TAP-ON: names = ["foo"]
+  %foo = seq.compreg %i0, %clock : i4
+  hw.output %foo : i4
+}

test/Dialect/Arc/infer-memories-names.mlir

@@ -0,0 +1,14 @@
+// RUN: circt-opt %s --arc-infer-memories="tap-memories=0" | FileCheck %s --check-prefixes=CHECK,CHECK-TAP-OFF
+// RUN: circt-opt %s --arc-infer-memories="tap-memories=1" | FileCheck %s --check-prefixes=CHECK,CHECK-TAP-ON
+
+// CHECK-LABEL: hw.module @TestMemory(
+hw.module @TestMemory(in %clock: !seq.clock, in %addr: i10, in %enable: i1, in %data: i8) {
+  // CHECK: arc.memory <1024 x i8, i10>
+  // CHECK-TAP-OFF-NOT: name = "foo"
+  // CHECK-TAP-ON: name = "foo"
+  hw.instance "foo" @Memory(W0_addr: %addr: i10, W0_en: %enable: i1, W0_clk: %clock: !seq.clock, W0_data: %data: i8) -> ()
+}
+
+hw.generator.schema @FIRRTLMem, "FIRRTL_Memory", ["depth", "numReadPorts", "numWritePorts", "numReadWritePorts", "readLatency", "writeLatency", "width", "maskGran", "readUnderWrite", "writeUnderWrite", "writeClockIDs"]
+hw.module.generated @Memory, @FIRRTLMem(in %W0_addr: i10, in %W0_en: i1, in %W0_clk: !seq.clock, in %W0_data: i8) attributes {depth = 1024 : i64, maskGran = 8 : ui32, numReadPorts = 0 : ui32, numReadWritePorts = 0 : ui32, numWritePorts = 1 : ui32, readLatency = 1 : ui32, readUnderWrite = 0 : ui32, width = 8 : ui32, writeClockIDs = [], writeLatency = 1 : ui32, writeUnderWrite = 1 : i32}
+

tools/arcilator/arcilator.cpp

@@ -89,6 +89,15 @@ static cl::opt<bool>
                        cl::desc("Make values with `sv.namehint` observable"),
                        cl::init(false), cl::cat(mainCategory));
 
+static cl::opt<bool> observeRegisters("observe-registers",
+                                      cl::desc("Make all registers observable"),
+                                      cl::init(false), cl::cat(mainCategory));
+
+static cl::opt<bool>
+    observeMemories("observe-memories",
+                    cl::desc("Make all memory contents observable"),
+                    cl::init(false), cl::cat(mainCategory));
+
 static cl::opt<std::string> stateFile("state-file", cl::desc("State file"),
                                       cl::value_desc("filename"), cl::init(""),
                                       cl::cat(mainCategory));
@@ -200,6 +209,7 @@ static void populatePipeline(PassManager &pm) {
   {
     arc::InferMemoriesOptions opts;
     opts.tapPorts = observePorts;
+    opts.tapMemories = observeMemories;
     pm.addPass(arc::createInferMemoriesPass(opts));
   }
   pm.addPass(createCSEPass());
@@ -208,7 +218,11 @@ static void populatePipeline(PassManager &pm) {
   // Restructure the input from a `hw.module` hierarchy to a collection of arcs.
   if (untilReached(UntilArcConversion))
     return;
-  pm.addPass(createConvertToArcsPass());
+  {
+    ConvertToArcsOptions opts;
+    opts.tapRegisters = observeRegisters;
+    pm.addPass(createConvertToArcsPass(opts));
+  }
   if (shouldDedup)
     pm.addPass(arc::createDedupPass());
   pm.addPass(arc::createInlineModulesPass());