[FIRRTL] Align inline layer ABI w/ spec proposal

Partially implement the inline layer convention ABI as described in the
FIRRTL Spec/ABI PR for this feature [[1]].  This changes the name of the
macro to be something that is more descriptive and cannot collide with
FIRRTL identifiers (which are supposed to not include `$`).

This does not implement the full ABI.  Namely, it does not correctly
handle circuits with multiple public modules.  (Notably, this isn't
implemented for the bind layer convention either.)  This will be fixed in
a follow-on.

[1]: chipsalliance/firrtl-spec#262

Signed-off-by: Schuyler Eldridge <[email protected]>

lib/Dialect/FIRRTL/Transforms/LowerLayers.cpp

@@ -121,8 +121,10 @@ static SmallString<32> fileNameForLayer(StringRef circuitName,
 
 /// For a layerblock `@A::@B::@C`, the verilog macro is `A_B_C`.
 static SmallString<32>
-macroNameForLayer(ArrayRef<FlatSymbolRefAttr> layerName) {
-  SmallString<32> result;
+macroNameForLayer(StringRef circuitName,
+                  ArrayRef<FlatSymbolRefAttr> layerName) {
+  SmallString<32> result("layer_");
+  result.append(circuitName);
   for (auto part : layerName)
     appendName(part, result, /*toLower=*/false,
                /*delimiter=*/Delimiter::InlineMacro);
@@ -182,8 +184,10 @@ class LowerLayersPass
   /// Preprocess layers to build macro declarations and cache information about
   /// the layers so that this can be quired later.
   void preprocessLayers(CircuitNamespace &ns, OpBuilder &b, LayerOp layer,
+                        StringRef circuitName,
                         SmallVector<FlatSymbolRefAttr> &stack);
-  void preprocessLayers(CircuitNamespace &ns, LayerOp layer);
+  void preprocessLayers(CircuitNamespace &ns, LayerOp layer,
+                        StringRef circuitName);
 
   /// Entry point for the function.
   void runOnOperation() override;
@@ -731,7 +735,7 @@ LogicalResult LowerLayersPass::runOnModuleBody(FModuleOp moduleOp,
 }
 
 void LowerLayersPass::preprocessLayers(CircuitNamespace &ns, OpBuilder &b,
-                                       LayerOp layer,
+                                       LayerOp layer, StringRef circuitName,
                                        SmallVector<FlatSymbolRefAttr> &stack) {
   stack.emplace_back(FlatSymbolRefAttr::get(layer.getSymNameAttr()));
   ArrayRef stackRef(stack);
@@ -740,7 +744,7 @@ void LowerLayersPass::preprocessLayers(CircuitNamespace &ns, OpBuilder &b,
        layer});
   if (layer.getConvention() == LayerConvention::Inline) {
     auto *ctx = &getContext();
-    auto macName = macroNameForLayer(stack);
+    auto macName = macroNameForLayer(circuitName, stack);
     auto symName = ns.newName(macName);
 
     auto symNameAttr = StringAttr::get(ctx, symName);
@@ -753,14 +757,15 @@ void LowerLayersPass::preprocessLayers(CircuitNamespace &ns, OpBuilder &b,
     macroNames[layer] = FlatSymbolRefAttr::get(&getContext(), symNameAttr);
   }
   for (auto child : layer.getOps<LayerOp>())
-    preprocessLayers(ns, b, child, stack);
+    preprocessLayers(ns, b, child, circuitName, stack);
   stack.pop_back();
 }
 
-void LowerLayersPass::preprocessLayers(CircuitNamespace &ns, LayerOp layer) {
+void LowerLayersPass::preprocessLayers(CircuitNamespace &ns, LayerOp layer,
+                                       StringRef circuitName) {
   OpBuilder b(layer);
   SmallVector<FlatSymbolRefAttr> stack;
-  preprocessLayers(ns, b, layer, stack);
+  preprocessLayers(ns, b, layer, circuitName, stack);
 }
 
 /// Process a circuit to remove all layer blocks in each module and top-level
@@ -770,6 +775,7 @@ void LowerLayersPass::runOnOperation() {
       llvm::dbgs() << "==----- Running LowerLayers "
                       "-------------------------------------------------===\n");
   CircuitOp circuitOp = getOperation();
+  StringRef circuitName = circuitOp.getName();
 
   // Initialize members which cannot be initialized automatically.
   llvm::sys::SmartMutex<true> mutex;
@@ -790,7 +796,7 @@ void LowerLayersPass::runOnOperation() {
     // Build verilog macro declarations for each inline layer declarations.
     // Cache layer symbol refs for lookup later.
     if (auto layerOp = dyn_cast<LayerOp>(op)) {
-      preprocessLayers(ns, layerOp);
+      preprocessLayers(ns, layerOp, circuitName);
       continue;
     }
   }
@@ -865,7 +871,6 @@ void LowerLayersPass::runOnOperation() {
   // TODO: This would be better handled without the use of verbatim ops.
   OpBuilder builder(circuitOp);
   SmallVector<std::pair<LayerOp, StringAttr>> layers;
-  StringRef circuitName = circuitOp.getName();
   circuitOp.walk<mlir::WalkOrder::PreOrder>([&](LayerOp layerOp) {
     auto parentOp = layerOp->getParentOfType<LayerOp>();
     while (!layers.empty() && parentOp != layers.back().first)

test/Dialect/FIRRTL/lower-layers.mlir

@@ -396,9 +396,9 @@ firrtl.circuit "Test" {
   // Inline Layers
   //===--------------------------------------------------------------------===//
 
-  // CHECK:      sv.macro.decl @[[INLINE:.*]]["Inline"]
-  // CHECK-NEXT: sv.macro.decl @Inline$Inline
-  // CHECK-NEXT: sv.macro.decl @Bound$Inline
+  // CHECK:      sv.macro.decl @layer_Test$Inline
+  // CHECK-NEXT: sv.macro.decl @layer_Test$Inline$Inline
+  // CHECK-NEXT: sv.macro.decl @layer_Test$Bound$Inline
   firrtl.layer @Inline inline {
     firrtl.layer @Inline inline {}
   }
@@ -409,15 +409,15 @@ firrtl.circuit "Test" {
 
   // CHECK:      firrtl.module private @ModuleWithInlineLayerBlocks_Bound() {
   // CHECK-NEXT:   %w3 = firrtl.wire
-  // CHECK-NEXT:   sv.ifdef @Bound$Inline {
+  // CHECK-NEXT:   sv.ifdef @layer_Test$Bound$Inline {
   // CHECK-NEXT:     %w4 = firrtl.wire
   // CHECK-NEXT:   }
   // CHECK-NEXT: }
 
   // CHECK-NEXT: firrtl.module @ModuleWithInlineLayerBlocks() {
-  // CHECK-NEXT:   sv.ifdef @[[INLINE]] {
+  // CHECK-NEXT:   sv.ifdef @layer_Test$Inline {
   // CHECK-NEXT:     %w1 = firrtl.wire
-  // CHECK-NEXT:     sv.ifdef @Inline$Inline {
+  // CHECK-NEXT:     sv.ifdef @layer_Test$Inline$Inline {
   // CHECK-NEXT:       %w2 = firrtl.wire
   // CHECK-NEXT:     }
   // CHECK-NEXT:   }