diff --git a/include/circt/Dialect/HW/HWOps.h b/include/circt/Dialect/HW/HWOps.h index 09ff52f9ac..eb0f372be8 100644 --- a/include/circt/Dialect/HW/HWOps.h +++ b/include/circt/Dialect/HW/HWOps.h @@ -103,6 +103,9 @@ FunctionType getModuleType(Operation *module); /// Returns the verilog module name attribute or symbol name of any module-like /// operations. StringAttr getVerilogModuleNameAttr(Operation *module); +inline StringRef getVerilogModuleName(Operation *module) { + return getVerilogModuleNameAttr(module).getValue(); +} /// Return the port name for the specified argument or result. These can only /// return a null StringAttr when the IR is invalid. diff --git a/lib/Conversion/ExportVerilog/ExportVerilog.cpp b/lib/Conversion/ExportVerilog/ExportVerilog.cpp index 492f0a33bd..730d68bb66 100644 --- a/lib/Conversion/ExportVerilog/ExportVerilog.cpp +++ b/lib/Conversion/ExportVerilog/ExportVerilog.cpp @@ -120,11 +120,8 @@ static bool isDuplicatableNullaryExpression(Operation *op) { static StringRef getSymOpName(Operation *symOp) { // Typeswitch of operation types which can define a symbol. return TypeSwitch(symOp) - .Case([&](HWModuleOp op) { return op.getName(); }) - .Case( - [&](HWModuleExternOp op) { return op.getVerilogModuleName(); }) - .Case( - [&](HWGeneratorSchemaOp op) { return op.sym_name(); }) + .Case( + [](Operation *op) { return getVerilogModuleName(op); }) .Case([&](InstanceOp op) { return op.getName().getValue(); }) .Case([&](WireOp op) { return op.name(); }) .Case([&](RegOp op) { return op.name(); }) @@ -2775,8 +2772,7 @@ LogicalResult StmtEmitter::visitStmt(InstanceOp op) { // Use the specified name or the symbol name as appropriate. auto *moduleOp = op.getReferencedModule(&state.symbolCache); assert(moduleOp && "Invalid IR"); - auto verilogName = getVerilogModuleNameAttr(moduleOp); - indent() << prefix << verilogName.getValue(); + indent() << prefix << getVerilogModuleName(moduleOp); // If this is a parameterized module, then emit the parameters. if (!op.parameters().empty()) { @@ -3348,7 +3344,7 @@ void ModuleEmitter::emitHWModule(HWModuleOp module) { SmallPtrSet moduleOpSet; moduleOpSet.insert(module); - os << "module " << module.getName(); + os << "module " << getVerilogModuleName(module); // If we have any parameters, print them on their own line. if (!module.parameters().empty()) { @@ -3743,7 +3739,7 @@ void SharedEmitterState::gatherFiles(bool separateModules) { // Emit into a separate file named after the module. if (attr || separateModules) - separateFile(mod, mod.getName() + ".sv"); + separateFile(mod, getVerilogModuleName(mod) + ".sv"); else rootFile.ops.push_back(info); }) diff --git a/lib/Conversion/ExportVerilog/LegalizeNames.cpp b/lib/Conversion/ExportVerilog/LegalizeNames.cpp index c94f06e687..4704bd124e 100644 --- a/lib/Conversion/ExportVerilog/LegalizeNames.cpp +++ b/lib/Conversion/ExportVerilog/LegalizeNames.cpp @@ -50,8 +50,6 @@ public: GlobalNameTable takeGlobalNameTable() { return std::move(globalNameTable); } - bool anythingChanged = false; - private: /// Check to see if the port names of the specified module conflict with /// keywords or themselves. If so, add the replacement names to @@ -74,11 +72,6 @@ private: /// Construct a GlobalNameResolver and do the initial scan to populate and /// unique the module/interfaces and port/parameter names. GlobalNameResolver::GlobalNameResolver(mlir::ModuleOp topLevel) { - // This symbol table is lazily constructed when global rewrites of module or - // interface member names are required. - mlir::SymbolTableCollection symbolTable; - Optional symbolUsers; - // Register the names of external modules which we cannot rename. This has to // occur in a first pass separate from the modules and interfaces which we are // actually allowed to rename, in order to ensure that we don't accidentally @@ -87,7 +80,7 @@ GlobalNameResolver::GlobalNameResolver(mlir::ModuleOp topLevel) { // Note that external modules *often* have name collisions, because they // correspond to the same verilog module with different parameters. if (isa(op) || isa(op)) { - auto name = hw::getVerilogModuleNameAttr(&op).getValue(); + auto name = getVerilogModuleNameAttr(&op).getValue(); if (!sv::isNameValid(name)) op.emitError("name \"") << name << "\" is not allowed in Verilog output"; @@ -95,30 +88,9 @@ GlobalNameResolver::GlobalNameResolver(mlir::ModuleOp topLevel) { } } - // If the module's symbol itself conflicts, then rename it and all uses of it. - auto legalizeSymbolName = [&](Operation *op, - NameCollisionResolver &resolver) { - StringAttr oldName = SymbolTable::getSymbolName(op); - auto newName = resolver.getLegalName(oldName); - if (newName == oldName.getValue()) - return; - - // Lazily construct the symbol table if it hasn't been built yet. - if (!symbolUsers.hasValue()) - symbolUsers.emplace(symbolTable, topLevel); - - // TODO: This is super inefficient, we should just rename the symbol as part - // of the other existing walks. - auto newNameAttr = StringAttr::get(topLevel.getContext(), newName); - symbolUsers->replaceAllUsesWith(op, newNameAttr); - SymbolTable::setSymbolName(op, newNameAttr); - anythingChanged = true; - }; - // Legalize module and interface names. for (auto &op : *topLevel.getBody()) { if (auto module = dyn_cast(op)) { - legalizeSymbolName(module, globalNameResolver); legalizeModuleNames(module); continue; } @@ -136,6 +108,14 @@ GlobalNameResolver::GlobalNameResolver(mlir::ModuleOp topLevel) { /// keywords or themselves. If so, add the replacement names to /// globalNameTable. void GlobalNameResolver::legalizeModuleNames(HWModuleOp module) { + // If the module's symbol itself conflicts, then set a "verilogName" attribute + // on the module to reflect the name we need to use. + StringRef oldName = module.getName(); + auto newName = globalNameResolver.getLegalName(oldName); + if (newName != oldName) + module->setAttr("verilogName", + StringAttr::get(module.getContext(), newName)); + NameCollisionResolver nameResolver; // Legalize the port names. diff --git a/lib/Dialect/SV/Transforms/HWExportModuleHierarchy.cpp b/lib/Dialect/SV/Transforms/HWExportModuleHierarchy.cpp index 7119968ada..5855b40c06 100644 --- a/lib/Dialect/SV/Transforms/HWExportModuleHierarchy.cpp +++ b/lib/Dialect/SV/Transforms/HWExportModuleHierarchy.cpp @@ -42,17 +42,16 @@ struct HWExportModuleHierarchyPass /// Recursively print the module hierarchy as serialized as JSON. static void printHierarchy(hw::InstanceOp &inst, SymbolTable &symbolTable, llvm::json::OStream &J) { + auto moduleOp = symbolTable.lookup(inst.moduleNameAttr().getValue()); + J.object([&] { J.attribute("instance_name", inst.instanceName()); - J.attribute("module_name", inst.moduleName()); + J.attribute("module_name", hw::getVerilogModuleName(moduleOp)); J.attributeArray("instances", [&] { - auto moduleOp = - symbolTable.lookup(inst.moduleNameAttr().getValue()); - // Only recurse on module ops, not extern or generated ops, whose internal // are opaque. - if (moduleOp) { - for (auto op : moduleOp.getOps()) { + if (auto module = dyn_cast(moduleOp)) { + for (auto op : module.getOps()) { printHierarchy(op, symbolTable, J); } } @@ -70,7 +69,7 @@ static void extractHierarchyFromTop(hw::HWModuleOp op, SymbolTable &symbolTable, // since the top-level module is not instantiated. J.object([&] { J.attribute("instance_name", op.getName()); - J.attribute("module_name", op.getName()); + J.attribute("module_name", hw::getVerilogModuleName(op)); J.attributeArray("instances", [&] { for (auto op : op.getOps()) printHierarchy(op, symbolTable, J); diff --git a/test/ExportVerilog/name-legalize.mlir b/test/ExportVerilog/name-legalize.mlir index 16d1228274..5695097d1a 100644 --- a/test/ExportVerilog/name-legalize.mlir +++ b/test/ExportVerilog/name-legalize.mlir @@ -160,8 +160,6 @@ hw.module @verbatim_renames(%a: i1) { sv.verbatim "// VERB Instance : {{0}} {{1}}" {symbols = [@struct, @wire1]} } - - // CHECK-LABEL: interface output_2; sv.interface @output { // CHECK-NEXT: logic input_0; diff --git a/test/ExportVerilog/sv-dialect.mlir b/test/ExportVerilog/sv-dialect.mlir index 4141aabeb2..ac6ae26b45 100644 --- a/test/ExportVerilog/sv-dialect.mlir +++ b/test/ExportVerilog/sv-dialect.mlir @@ -815,11 +815,11 @@ hw.module @verbatim_M1(%clock : i1, %cond : i1, %val : i8) { %c42_2 = hw.constant 42 : i8 %xor = comb.xor %val, %c42_2 : i8 hw.instance "aa1" sym @verbatim_b1 @verbatim_inout_2() ->() - // CHECK: MACRO(val + 8'h2A, val ^ 8'h2A reg=reg1, verbatim_M2, verbatim_inout_2, verbatim_schema~aa1,reg2 = reg2 ) - sv.verbatim "MACRO({{0}}, {{1}} reg={{2}}, {{3}}, {{4}}, {{5}}~{{6}},reg2 = {{7}} )" + // CHECK: MACRO(val + 8'h2A, val ^ 8'h2A reg=reg1, verbatim_M2, verbatim_inout_2, aa1,reg2 = reg2 ) + sv.verbatim "MACRO({{0}}, {{1}} reg={{2}}, {{3}}, {{4}}, {{5}},reg2 = {{6}} )" (%add, %xor) : i8,i8 {symbols = [@verbatim_reg1, @verbatim_M2, - @verbatim_inout_2, @verbatim_schema, @verbatim_b1, @verbatim_reg2]} + @verbatim_inout_2, @verbatim_b1, @verbatim_reg2]} // CHECK: Wire : wire25 sv.verbatim " Wire : {{0}}" {symbols = [@verbatim_wireSym1]} }