[ExportVerilog][HW] Introduce HWEmittableModuleLike interface and use…

… it for Prepare, NFC (#7004) This changes pre-passes for ExportVerilog to run on HWEmittableModuleLike instead of HWModuleOp. #6977 is going to add a support for SV func op and legalization needed for SV func as well. HWEmittableModuleLike is a new interface that inherits Emittable+HWModuleLike. I considered to use a trait but we cannot use a trait for pass scheduling so an interface is used.
llvm · Jun 6, 2024 · 927a376 · 927a376 · uenoku · Jun 6, 2024
1 parent bf24871
commit 927a376
Show file tree

Hide file tree

Showing 16 changed files with 46 additions and 36 deletions.
diff --git a/include/circt/Conversion/ExportVerilog.h b/include/circt/Conversion/ExportVerilog.h
@@ -19,6 +19,7 @@
 namespace circt {
 namespace hw {
 class HWModuleLike;
+class HWEmittableModuleLike;
 } // namespace hw
 
 std::unique_ptr<mlir::Pass>

diff --git a/include/circt/Conversion/Passes.td b/include/circt/Conversion/Passes.td
@@ -118,8 +118,8 @@ def HWLowerInstanceChoices : Pass<"hw-lower-instance-choices",
   ];
 }
 
-def PrepareForEmission : Pass<"prepare-for-emission",
-                              "hw::HWModuleOp"> {
+def PrepareForEmission : InterfacePass<"prepare-for-emission",
+                                       "hw::HWEmittableModuleLike"> {
   let summary = "Prepare IR for ExportVerilog";
   let description = [{
     This pass runs only PrepareForEmission.

diff --git a/include/circt/Dialect/Emit/EmitOpInterfaces.h b/include/circt/Dialect/Emit/EmitOpInterfaces.h
@@ -16,15 +16,6 @@
 #include "circt/Support/LLVM.h"
 #include "mlir/IR/OpDefinition.h"
 
-namespace circt {
-namespace emit {
-
-template <typename ConcreteType>
-class Emittable : public OpTrait::TraitBase<ConcreteType, Emittable> {};
-
-} // namespace emit
-} // namespace circt
-
 #include "circt/Dialect/Emit/EmitOpInterfaces.h.inc"
 
 #endif // CIRCT_DIALECT_EMIT_EMITOPINTERFACES_H
diff --git a/include/circt/Dialect/Emit/EmitOpInterfaces.td b/include/circt/Dialect/Emit/EmitOpInterfaces.td
@@ -15,7 +15,7 @@
 
 include "mlir/IR/OpBase.td"
 
-def Emittable : NativeOpTrait<"Emittable"> {
+def Emittable : OpInterface<"Emittable", []> {
   let cppNamespace = "::circt::emit";
 }
 

diff --git a/include/circt/Dialect/HW/HWOpInterfaces.h b/include/circt/Dialect/HW/HWOpInterfaces.h
@@ -13,6 +13,7 @@
 #ifndef CIRCT_DIALECT_HW_HWOPINTERFACES_H
 #define CIRCT_DIALECT_HW_HWOPINTERFACES_H
 
+#include "circt/Dialect/Emit/EmitOpInterfaces.h"
 #include "circt/Dialect/HW/HWInstanceImplementation.h"
 #include "circt/Dialect/HW/HWTypes.h"
 #include "circt/Dialect/HW/InnerSymbolTable.h"

diff --git a/include/circt/Dialect/HW/HWOpInterfaces.td b/include/circt/Dialect/HW/HWOpInterfaces.td
@@ -15,6 +15,7 @@
 
 include "mlir/IR/SymbolInterfaces.td"
 include "mlir/IR/OpBase.td"
+include "circt/Dialect/Emit/EmitOpInterfaces.td"
 include "circt/Support/InstanceGraphInterface.td"
 
 def PortList : OpInterface<"PortList", []> {
@@ -402,6 +403,15 @@ def HWMutableModuleLike : OpInterface<"HWMutableModuleLike", [HWModuleLike]> {
   ];
 }
 
+def HWEmittableModuleLike : OpInterface<"HWEmittableModuleLike", [HWModuleLike,
+                                        Emittable]> {
+  let cppNamespace = "::circt::hw";
+  let description = [{
+    This interface indicates that the module like op is emittable in SV and
+    requires SV legalization on its body.
+  }];
+}
+
 
 def HWInstanceLike : OpInterface<"HWInstanceLike", [
     InstanceGraphInstanceOpInterface]> {

diff --git a/include/circt/Dialect/HW/HWStructure.td b/include/circt/Dialect/HW/HWStructure.td
@@ -114,7 +114,7 @@ class HWModuleOpBase<string mnemonic, list<Trait> traits = []> :
 def HWModuleOp : HWModuleOpBase<"module",
       [IsolatedFromAbove, RegionKindInterface,
        SingleBlockImplicitTerminator<"OutputOp">,
-       Emittable]>{
+       HWEmittableModuleLike]>{
   let summary = "HW Module";
   let description = [{
     The "hw.module" operation represents a Verilog module, including a given

diff --git a/include/circt/Dialect/SV/SVStatements.td b/include/circt/Dialect/SV/SVStatements.td
@@ -907,7 +907,8 @@ def FuncOp : SVOp<"func",
      [IsolatedFromAbove, Symbol, OpAsmOpInterface, ProceduralRegion,
       DeclareOpInterfaceMethods<HWModuleLike>,
       DeclareOpInterfaceMethods<PortList>,
-      FunctionOpInterface, HasParent<"mlir::ModuleOp">]> {
+      FunctionOpInterface, HasParent<"mlir::ModuleOp">, 
+      HWEmittableModuleLike]> {
   let summary = "A SystemVerilog function";
   let description = [{
     `sv.func` represents SystemVerilog function in IEEE 1800-2017 section 13.4

diff --git a/lib/Conversion/ExportVerilog/ExportVerilog.cpp b/lib/Conversion/ExportVerilog/ExportVerilog.cpp
@@ -1413,7 +1413,8 @@ StringAttr ExportVerilog::inferStructuralNameForTemporary(Value expr) {
 
   // Module ports carry names!
   if (auto blockArg = dyn_cast<BlockArgument>(expr)) {
-    auto moduleOp = cast<HWModuleOp>(blockArg.getOwner()->getParentOp());
+    auto moduleOp =
+        cast<HWEmittableModuleLike>(blockArg.getOwner()->getParentOp());
     StringRef name = getPortVerilogName(moduleOp, blockArg.getArgNumber());
     result = StringAttr::get(expr.getContext(), name);
 
@@ -6311,7 +6312,7 @@ void FileEmitter::emit(emit::FileListOp op) {
 void FileEmitter::emitOp(emit::RefOp op) {
   StringAttr target = op.getTargetAttr().getAttr();
   auto *targetOp = state.symbolCache.getDefinition(target);
-  assert(targetOp->hasTrait<emit::Emittable>() && "target must be emittable");
+  assert(isa<emit::Emittable>(targetOp) && "target must be emittable");
 
   TypeSwitch<Operation *>(targetOp)
       .Case<hw::HWModuleOp>(
@@ -6782,8 +6783,9 @@ LogicalResult circt::exportVerilog(ModuleOp module, llvm::raw_ostream &os) {
   LoweringOptions options(module);
   if (failed(lowerHWInstanceChoices(module)))
     return failure();
-  SmallVector<HWModuleOp> modulesToPrepare;
-  module.walk([&](HWModuleOp op) { modulesToPrepare.push_back(op); });
+  SmallVector<HWEmittableModuleLike> modulesToPrepare;
+  module.walk(
+      [&](HWEmittableModuleLike op) { modulesToPrepare.push_back(op); });
   if (failed(failableParallelForEach(
           module->getContext(), modulesToPrepare,
           [&](auto op) { return prepareHWModule(op, options); })))
@@ -6800,7 +6802,7 @@ struct ExportVerilogPass : public ExportVerilogBase<ExportVerilogPass> {
     mlir::OpPassManager preparePM("builtin.module");
     preparePM.addPass(createLegalizeAnonEnumsPass());
     preparePM.addPass(createHWLowerInstanceChoicesPass());
-    auto &modulePM = preparePM.nest<hw::HWModuleOp>();
+    auto &modulePM = preparePM.nestAny();
     modulePM.addPass(createPrepareForEmissionPass());
     if (failed(runPipeline(preparePM, getOperation())))
       return signalPassFailure();
@@ -6959,8 +6961,9 @@ LogicalResult circt::exportSplitVerilog(ModuleOp module, StringRef dirname) {
   LoweringOptions options(module);
   if (failed(lowerHWInstanceChoices(module)))
     return failure();
-  SmallVector<HWModuleOp> modulesToPrepare;
-  module.walk([&](HWModuleOp op) { modulesToPrepare.push_back(op); });
+  SmallVector<HWEmittableModuleLike> modulesToPrepare;
+  module.walk(
+      [&](HWEmittableModuleLike op) { modulesToPrepare.push_back(op); });
   if (failed(failableParallelForEach(
           module->getContext(), modulesToPrepare,
           [&](auto op) { return prepareHWModule(op, options); })))

diff --git a/lib/Conversion/ExportVerilog/ExportVerilogInternals.h b/lib/Conversion/ExportVerilog/ExportVerilogInternals.h
@@ -452,10 +452,10 @@ LogicalResult lowerHWInstanceChoices(mlir::ModuleOp module);
 /// For each module we emit, do a prepass over the structure, pre-lowering and
 /// otherwise rewriting operations we don't want to emit.
 LogicalResult prepareHWModule(Block &block, const LoweringOptions &options);
-LogicalResult prepareHWModule(hw::HWModuleOp module,
+LogicalResult prepareHWModule(hw::HWEmittableModuleLike module,
                               const LoweringOptions &options);
 
-void pruneZeroValuedLogic(hw::HWModuleOp module);
+void pruneZeroValuedLogic(hw::HWEmittableModuleLike module);
 
 /// Rewrite module names and interfaces to not conflict with each other or with
 /// Verilog keywords.

diff --git a/lib/Conversion/ExportVerilog/LegalizeNames.cpp b/lib/Conversion/ExportVerilog/LegalizeNames.cpp
@@ -138,17 +138,18 @@ class GlobalNameResolver {
 } // namespace circt
 
 // This function legalizes local names in the given module.
-static void legalizeModuleLocalNames(HWModuleOp module,
+static void legalizeModuleLocalNames(HWEmittableModuleLike module,
                                      const LoweringOptions &options,
                                      const GlobalNameTable &globalNameTable) {
   // A resolver for a local name collison.
   NameCollisionResolver nameResolver(options);
   globalNameTable.addReservedNames(nameResolver);
 
   // Register names used by parameters.
-  for (auto param : module.getParameters())
-    nameResolver.insertUsedName(globalNameTable.getParameterVerilogName(
-        module, cast<ParamDeclAttr>(param).getName()));
+  if (auto hwModule = dyn_cast<hw::HWModuleOp>(*module))
+    for (auto param : hwModule.getParameters())
+      nameResolver.insertUsedName(globalNameTable.getParameterVerilogName(
+          module, cast<ParamDeclAttr>(param).getName()));
 
   auto *ctxt = module.getContext();
 
@@ -183,7 +184,7 @@ static void legalizeModuleLocalNames(HWModuleOp module,
   // Legalize the value names. We first mark existing hw.verilogName attrs as
   // being used, and then resolve names of declarations.
   module.walk([&](Operation *op) {
-    if (!isa<HWModuleOp>(op)) {
+    if (module != op) {
       // If there is a hw.verilogName attr, mark names as used.
       if (auto name = op->getAttrOfType<StringAttr>(verilogNameAttr)) {
         nameResolver.insertUsedName(
@@ -274,7 +275,8 @@ GlobalNameResolver::GlobalNameResolver(mlir::ModuleOp topLevel,
 
   // Legalize names in HW modules parallelly.
   mlir::parallelForEach(
-      topLevel.getContext(), topLevel.getOps<HWModuleOp>(), [&](auto module) {
+      topLevel.getContext(), topLevel.getOps<HWEmittableModuleLike>(),
+      [&](auto module) {
         legalizeModuleLocalNames(module, options, globalNameTable);
       });
 

diff --git a/lib/Conversion/ExportVerilog/PrepareForEmission.cpp b/lib/Conversion/ExportVerilog/PrepareForEmission.cpp
@@ -1247,7 +1247,7 @@ static LogicalResult legalizeHWModule(Block &block,
 }
 
 // NOLINTNEXTLINE(misc-no-recursion)
-LogicalResult ExportVerilog::prepareHWModule(hw::HWModuleOp module,
+LogicalResult ExportVerilog::prepareHWModule(hw::HWEmittableModuleLike module,
                                              const LoweringOptions &options) {
   // Zero-valued logic pruning.
   pruneZeroValuedLogic(module);
@@ -1267,7 +1267,7 @@ namespace {
 struct PrepareForEmissionPass
     : public PrepareForEmissionBase<PrepareForEmissionPass> {
   void runOnOperation() override {
-    HWModuleOp module = getOperation();
+    auto module = getOperation();
     LoweringOptions options(cast<mlir::ModuleOp>(module->getParentOp()));
     if (failed(prepareHWModule(module, options)))
       signalPassFailure();

diff --git a/lib/Conversion/ExportVerilog/PruneZeroValuedLogic.cpp b/lib/Conversion/ExportVerilog/PruneZeroValuedLogic.cpp
@@ -241,9 +241,9 @@ static void addNoI0ResultPruningPattern(ConversionTarget &target,
 
 } // namespace
 
-void ExportVerilog::pruneZeroValuedLogic(hw::HWModuleOp module) {
-  ConversionTarget target(*module.getContext());
-  RewritePatternSet patterns(module.getContext());
+void ExportVerilog::pruneZeroValuedLogic(HWEmittableModuleLike module) {
+  ConversionTarget target(*module->getContext());
+  RewritePatternSet patterns(module->getContext());
   PruneTypeConverter typeConverter;
 
   target.addLegalDialect<sv::SVDialect, comb::CombDialect, hw::HWDialect>();

diff --git a/lib/Conversion/PassDetail.h b/lib/Conversion/PassDetail.h
@@ -108,6 +108,7 @@ class EmitDialect;
 namespace hw {
 class HWDialect;
 class HWModuleOp;
+class HWEmittableModuleLike;
 } // namespace hw
 
 namespace hwarith {

diff --git a/lib/Dialect/Emit/EmitOps.cpp b/lib/Dialect/Emit/EmitOps.cpp
@@ -74,7 +74,7 @@ LogicalResult RefOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
   auto *op = symbolTable.lookupNearestSymbolFrom(getOperation(), target);
   if (!op)
     return emitError("invalid symbol reference: ") << target;
-  if (!op->hasTrait<emit::Emittable>())
+  if (!isa<emit::Emittable>(op))
     return emitError("does not target an emittable op: ") << target;
   return success();
 }

diff --git a/test/Conversion/ExportVerilog/prepare-for-emission.mlir b/test/Conversion/ExportVerilog/prepare-for-emission.mlir
@@ -1,4 +1,4 @@
-// RUN: circt-opt %s -prepare-for-emission --split-input-file -verify-diagnostics | FileCheck %s
+// RUN: circt-opt %s --pass-pipeline='builtin.module(any(prepare-for-emission))' --split-input-file -verify-diagnostics | FileCheck %s
 // RUN: circt-opt %s -export-verilog -split-input-file
 
 // CHECK: @namehint_variadic