Revert "[ExportVerilog][HW] Introduce HWEmittableModuleLike interface…

… and use it for Prepare, NFC (#7004)" This reverts commit 927a376.
llvm · Jun 7, 2024 · 37cb7f5 · 37cb7f5
1 parent 69085ea
commit 37cb7f5
Show file tree

Hide file tree

Showing 16 changed files with 36 additions and 46 deletions.
diff --git a/include/circt/Conversion/ExportVerilog.h b/include/circt/Conversion/ExportVerilog.h
@@ -19,7 +19,6 @@
 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 : InterfacePass<"prepare-for-emission",
-                                       "hw::HWEmittableModuleLike"> {
+def PrepareForEmission : Pass<"prepare-for-emission",
+                              "hw::HWModuleOp"> {
   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,6 +16,15 @@
 #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 : OpInterface<"Emittable", []> {
+def Emittable : NativeOpTrait<"Emittable"> {
   let cppNamespace = "::circt::emit";
 }
 

diff --git a/include/circt/Dialect/HW/HWOpInterfaces.h b/include/circt/Dialect/HW/HWOpInterfaces.h
@@ -13,7 +13,6 @@
 #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,7 +15,6 @@
 
 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", []> {
@@ -403,15 +402,6 @@ 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">,
-       HWEmittableModuleLike]>{
+       Emittable]>{
   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,8 +907,7 @@ def FuncOp : SVOp<"func",
      [IsolatedFromAbove, Symbol, OpAsmOpInterface, ProceduralRegion,
       DeclareOpInterfaceMethods<HWModuleLike>,
       DeclareOpInterfaceMethods<PortList>,
-      FunctionOpInterface, HasParent<"mlir::ModuleOp">, 
-      HWEmittableModuleLike]> {
+      FunctionOpInterface, HasParent<"mlir::ModuleOp">]> {
   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,8 +1413,7 @@ StringAttr ExportVerilog::inferStructuralNameForTemporary(Value expr) {
 
   // Module ports carry names!
   if (auto blockArg = dyn_cast<BlockArgument>(expr)) {
-    auto moduleOp =
-        cast<HWEmittableModuleLike>(blockArg.getOwner()->getParentOp());
+    auto moduleOp = cast<HWModuleOp>(blockArg.getOwner()->getParentOp());
     StringRef name = getPortVerilogName(moduleOp, blockArg.getArgNumber());
     result = StringAttr::get(expr.getContext(), name);
 
@@ -6312,7 +6311,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(isa<emit::Emittable>(targetOp) && "target must be emittable");
+  assert(targetOp->hasTrait<emit::Emittable>() && "target must be emittable");
 
   TypeSwitch<Operation *>(targetOp)
       .Case<hw::HWModuleOp>(
@@ -6783,9 +6782,8 @@ LogicalResult circt::exportVerilog(ModuleOp module, llvm::raw_ostream &os) {
   LoweringOptions options(module);
   if (failed(lowerHWInstanceChoices(module)))
     return failure();
-  SmallVector<HWEmittableModuleLike> modulesToPrepare;
-  module.walk(
-      [&](HWEmittableModuleLike op) { modulesToPrepare.push_back(op); });
+  SmallVector<HWModuleOp> modulesToPrepare;
+  module.walk([&](HWModuleOp op) { modulesToPrepare.push_back(op); });
   if (failed(failableParallelForEach(
           module->getContext(), modulesToPrepare,
           [&](auto op) { return prepareHWModule(op, options); })))
@@ -6802,7 +6800,7 @@ struct ExportVerilogPass : public ExportVerilogBase<ExportVerilogPass> {
     mlir::OpPassManager preparePM("builtin.module");
     preparePM.addPass(createLegalizeAnonEnumsPass());
     preparePM.addPass(createHWLowerInstanceChoicesPass());
-    auto &modulePM = preparePM.nestAny();
+    auto &modulePM = preparePM.nest<hw::HWModuleOp>();
     modulePM.addPass(createPrepareForEmissionPass());
     if (failed(runPipeline(preparePM, getOperation())))
       return signalPassFailure();
@@ -6961,9 +6959,8 @@ LogicalResult circt::exportSplitVerilog(ModuleOp module, StringRef dirname) {
   LoweringOptions options(module);
   if (failed(lowerHWInstanceChoices(module)))
     return failure();
-  SmallVector<HWEmittableModuleLike> modulesToPrepare;
-  module.walk(
-      [&](HWEmittableModuleLike op) { modulesToPrepare.push_back(op); });
+  SmallVector<HWModuleOp> modulesToPrepare;
+  module.walk([&](HWModuleOp 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::HWEmittableModuleLike module,
+LogicalResult prepareHWModule(hw::HWModuleOp module,
                               const LoweringOptions &options);
 
-void pruneZeroValuedLogic(hw::HWEmittableModuleLike module);
+void pruneZeroValuedLogic(hw::HWModuleOp 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,18 +138,17 @@ class GlobalNameResolver {
 } // namespace circt
 
 // This function legalizes local names in the given module.
-static void legalizeModuleLocalNames(HWEmittableModuleLike module,
+static void legalizeModuleLocalNames(HWModuleOp 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.
-  if (auto hwModule = dyn_cast<hw::HWModuleOp>(*module))
-    for (auto param : hwModule.getParameters())
-      nameResolver.insertUsedName(globalNameTable.getParameterVerilogName(
-          module, cast<ParamDeclAttr>(param).getName()));
+  for (auto param : module.getParameters())
+    nameResolver.insertUsedName(globalNameTable.getParameterVerilogName(
+        module, cast<ParamDeclAttr>(param).getName()));
 
   auto *ctxt = module.getContext();
 
@@ -184,7 +183,7 @@ static void legalizeModuleLocalNames(HWEmittableModuleLike 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 (module != op) {
+    if (!isa<HWModuleOp>(op)) {
       // If there is a hw.verilogName attr, mark names as used.
       if (auto name = op->getAttrOfType<StringAttr>(verilogNameAttr)) {
         nameResolver.insertUsedName(
@@ -275,8 +274,7 @@ GlobalNameResolver::GlobalNameResolver(mlir::ModuleOp topLevel,
 
   // Legalize names in HW modules parallelly.
   mlir::parallelForEach(
-      topLevel.getContext(), topLevel.getOps<HWEmittableModuleLike>(),
-      [&](auto module) {
+      topLevel.getContext(), topLevel.getOps<HWModuleOp>(), [&](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::HWEmittableModuleLike module,
+LogicalResult ExportVerilog::prepareHWModule(hw::HWModuleOp module,
                                              const LoweringOptions &options) {
   // Zero-valued logic pruning.
   pruneZeroValuedLogic(module);
@@ -1267,7 +1267,7 @@ namespace {
 struct PrepareForEmissionPass
     : public PrepareForEmissionBase<PrepareForEmissionPass> {
   void runOnOperation() override {
-    auto module = getOperation();
+    HWModuleOp 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(HWEmittableModuleLike module) {
-  ConversionTarget target(*module->getContext());
-  RewritePatternSet patterns(module->getContext());
+void ExportVerilog::pruneZeroValuedLogic(hw::HWModuleOp 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,7 +108,6 @@ 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 (!isa<emit::Emittable>(op))
+  if (!op->hasTrait<emit::Emittable>())
     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 --pass-pipeline='builtin.module(any(prepare-for-emission))' --split-input-file -verify-diagnostics | FileCheck %s
+// RUN: circt-opt %s -prepare-for-emission --split-input-file -verify-diagnostics | FileCheck %s
 // RUN: circt-opt %s -export-verilog -split-input-file
 
 // CHECK: @namehint_variadic