From 12997517933e49a3218aa28b0cfc48477b89a762 Mon Sep 17 00:00:00 2001 From: woshiren Date: Fri, 22 May 2026 23:10:23 +0800 Subject: [PATCH 1/7] [FIRRTLToHW] Remove the SYNTHESIS guard in printf/flush lowering path --- lib/Conversion/FIRRTLToHW/LowerToHW.cpp | 63 ++++++++++----------- lib/Conversion/Utils/SVLoweringUtils.cpp | 4 ++ test/Conversion/FIRRTLToHW/lower-to-hw.mlir | 4 +- test/firtool/lower-layers.fir | 2 - 4 files changed, 34 insertions(+), 39 deletions(-) diff --git a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp index e0245a2cfe54..1a92fd8601e6 100644 --- a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp +++ b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp @@ -2963,43 +2963,38 @@ FIRRTLLowering::lowerSimFormatString(StringRef originalFormatString, LogicalResult FIRRTLLowering::lowerStatementWithFd( const FileDescriptorInfo &fileDescriptor, Value clock, Value cond, const std::function &fn, bool usePrintfCond) { - // Emit an "#ifndef SYNTHESIS" guard into the always block. bool failed = false; - circuitState.addMacroDecl(builder.getStringAttr("SYNTHESIS")); - addToIfDefBlock("SYNTHESIS", std::function(), [&]() { - addToAlwaysBlock(clock, [&]() { - // TODO: This is not printf specific anymore. Replace "Printf" with "FD" - // or similar but be aware that changing macro name breaks existing uses. - circuitState.usedPrintf = true; - if (usePrintfCond) - circuitState.addFragment(theModule, "PRINTF_COND_FRAGMENT"); - - // Emit an "sv.if '`PRINTF_COND_ & cond' into the #ifndef. - Value ifCond = cond; - if (usePrintfCond) { - ifCond = - sv::MacroRefExprOp::create(builder, cond.getType(), "PRINTF_COND_"); - ifCond = builder.createOrFold(ifCond, cond, true); - } + addToAlwaysBlock(clock, [&]() { + // TODO: This is not printf specific anymore. Replace "Printf" with "FD" + // or similar but be aware that changing macro name breaks existing uses. + circuitState.usedPrintf = true; + if (usePrintfCond) + circuitState.addFragment(theModule, "PRINTF_COND_FRAGMENT"); + + Value ifCond = cond; + if (usePrintfCond) { + ifCond = + sv::MacroRefExprOp::create(builder, cond.getType(), "PRINTF_COND_"); + ifCond = builder.createOrFold(ifCond, cond, true); + } - addIfProceduralBlock(ifCond, [&]() { - // `fd`represents a file decriptor. Use the stdout or the one opened - // using $fopen. - Value fd; - if (fileDescriptor.isDefaultFd()) { - // Emit the sv.fwrite, writing to stderr by default. - fd = hw::ConstantOp::create(builder, APInt(32, 0x80000002)); - } else { - // Call the library function to get the FD. - auto fdOrError = callFileDescriptorLib(fileDescriptor); - if (llvm::failed(fdOrError)) { - failed = true; - return; - } - fd = *fdOrError; + addIfProceduralBlock(ifCond, [&]() { + // `fd`represents a file decriptor. Use the stdout or the one opened + // using $fopen. + Value fd; + if (fileDescriptor.isDefaultFd()) { + // Emit the sv.fwrite, writing to stderr by default. + fd = hw::ConstantOp::create(builder, APInt(32, 0x80000002)); + } else { + // Call the library function to get the FD. + auto fdOrError = callFileDescriptorLib(fileDescriptor); + if (llvm::failed(fdOrError)) { + failed = true; + return; } - failed = llvm::failed(fn(fd)); - }); + fd = *fdOrError; + } + failed = llvm::failed(fn(fd)); }); }); return failure(failed); diff --git a/lib/Conversion/Utils/SVLoweringUtils.cpp b/lib/Conversion/Utils/SVLoweringUtils.cpp index c748fbf2034c..0fdb403e1ef5 100644 --- a/lib/Conversion/Utils/SVLoweringUtils.cpp +++ b/lib/Conversion/Utils/SVLoweringUtils.cpp @@ -63,6 +63,7 @@ void sv::emitFileDescriptorRuntime(Operation *fileScopeOp, auto fragmentSymName = ::getFileDescriptorFragmentSymName(builder.getContext()); auto macroSymName = builder.getStringAttr(kFileDescriptorMacroName); + auto synthesisSymName = builder.getStringAttr("SYNTHESIS"); SymbolTable symbolTable(fileScopeOp); auto emitGuard = [&](StringRef guard, llvm::function_ref body) { @@ -95,6 +96,9 @@ void sv::emitFileDescriptorRuntime(Operation *fileScopeOp, symbolTable.insert(func); } + if (!symbolTable.lookup(synthesisSymName)) + symbolTable.insert(sv::MacroDeclOp::create(builder, synthesisSymName)); + if (!symbolTable.lookup(macroSymName)) symbolTable.insert(sv::MacroDeclOp::create(builder, macroSymName)); diff --git a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir index 22a3955e665b..e9475c2405bd 100644 --- a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir +++ b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir @@ -362,9 +362,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // CHECK: [[ADDSIGNED:%.+]] = comb.add - // CHECK: sv.ifdef @SYNTHESIS { - // CHECK-NEXT: } else { - // CHECK-NEXT: sv.always posedge [[CLOCK]] { + // CHECK: sv.always posedge [[CLOCK]] { // CHECK-NEXT: %[[PRINTF_COND:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND]], %reset // CHECK-NEXT: sv.if [[AND]] { diff --git a/test/firtool/lower-layers.fir b/test/firtool/lower-layers.fir index 78c32fed8303..e4e4c8823cd2 100644 --- a/test/firtool/lower-layers.fir +++ b/test/firtool/lower-layers.fir @@ -127,12 +127,10 @@ circuit TestHarness: define trace = x ; CHECK: module TestHarness_Verification() - ; CHECK: `ifndef SYNTHESIS ; CHECK: always @(posedge TestHarness.clock) begin ; CHECK: if ((`PRINTF_COND_) & TestHarness.reset) ; CHECK: $fwrite(32'h80000002, "The last PC was: %x", TestHarness.dut.verification.pc_d); ; CHECK: end // always @(posedge) - ; CHECK: `endif // not def SYNTHESIS ; CHECK: endmodule ; CHECK: module TestHarness( From e7c2c5b0987efe487f1bef86cd2ccbb3c66068d8 Mon Sep 17 00:00:00 2001 From: woshiren Date: Sat, 23 May 2026 19:18:48 +0800 Subject: [PATCH 2/7] fix indentation --- test/Conversion/FIRRTLToHW/lower-to-hw.mlir | 114 ++++++++++---------- test/firtool/lower-layers.fir | 8 +- 2 files changed, 61 insertions(+), 61 deletions(-) diff --git a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir index e9475c2405bd..01cf29674a42 100644 --- a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir +++ b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir @@ -362,63 +362,63 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // CHECK: [[ADDSIGNED:%.+]] = comb.add - // CHECK: sv.always posedge [[CLOCK]] { - // CHECK-NEXT: %[[PRINTF_COND:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 - // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND]], %reset - // CHECK-NEXT: sv.if [[AND]] { - // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 - // CHECK-NEXT: sv.fwrite %[[STDERR]], "No operands and literal: %%\0A" - // CHECK-NEXT: } - // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 - // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 - // CHECK-NEXT: sv.if [[AND]] { - // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 - // CHECK-NEXT: sv.fwrite %[[STDERR]], "Binary: %b %0b %4b\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5 - // CHECK-NEXT: } - // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 - // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 - // CHECK-NEXT: sv.if [[AND]] { - // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 - // CHECK-NEXT: sv.fwrite %[[STDERR]], "Decimal: %d %0d %4d\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5 - // CHECK-NEXT: } - // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 - // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 - // CHECK-NEXT: sv.if [[AND]] { - // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 - // CHECK-NEXT: sv.fwrite %[[STDERR]], "Hexadecimal: %x %0x %4x\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5 - // CHECK-NEXT: } - // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 - // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 - // CHECK-NEXT: sv.if [[AND]] { - // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 - // CHECK-NEXT: sv.fwrite %[[STDERR]], "ASCII Character: %c\0A"([[ADD]]) : i5 - // CHECK-NEXT: } - // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 - // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 - // CHECK-NEXT: sv.if [[AND]] { - // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 - // CHECK-NEXT: [[SUMSIGNED:%.+]] = sv.system "signed"([[ADDSIGNED]]) - // CHECK-NEXT: [[DSIGNED:%.+]] = sv.system "signed"(%d) - // CHECK-NEXT: sv.fwrite %[[STDERR]], "Hi signed %d %d\0A"([[SUMSIGNED]], [[DSIGNED]]) : i5, i4 - // CHECK-NEXT: } - // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 - // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 - // CHECK-NEXT: sv.if [[AND]] { - // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 - // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 - // CHECK-NEXT: sv.fwrite %[[STDERR]], "[%0t]: %d %m"([[TIME]], %a) : i64, i4 - // CHECK-NEXT: } - // CHECK-NEXT: sv.if %reset { - // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 - // CHECK-NEXT: [[STR:%.+]] = sv.sformatf "%0t%d.txt"([[TIME]], %a) : i64, i4 - // CHECK-NEXT: [[FD:%.+]] = sv.func.call.procedural @"__circt_lib_logging::FileDescriptor::get"([[STR]]) : (!hw.string) -> i32 - // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 - // CHECK-NEXT: sv.fwrite [[FD]], "[%0t]: dynamic file name\0A"([[TIME]]) : i64 - // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 - // CHECK-NEXT: [[STR:%.+]] = sv.sformatf "%0t%d.txt"([[TIME]], %a) : i64, i4 - // CHECK-NEXT: [[FD:%.+]] = sv.func.call.procedural @"__circt_lib_logging::FileDescriptor::get"([[STR]]) : (!hw.string) -> i32 - // CHECK-NEXT: sv.fflush fd [[FD]] - // CHECK-NEXT: } + // CHECK: sv.always posedge [[CLOCK]] { + // CHECK-NEXT: %[[PRINTF_COND:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 + // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND]], %reset + // CHECK-NEXT: sv.if [[AND]] { + // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 + // CHECK-NEXT: sv.fwrite %[[STDERR]], "No operands and literal: %%\0A" + // CHECK-NEXT: } + // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 + // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 + // CHECK-NEXT: sv.if [[AND]] { + // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 + // CHECK-NEXT: sv.fwrite %[[STDERR]], "Binary: %b %0b %4b\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5 + // CHECK-NEXT: } + // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 + // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 + // CHECK-NEXT: sv.if [[AND]] { + // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 + // CHECK-NEXT: sv.fwrite %[[STDERR]], "Decimal: %d %0d %4d\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5 + // CHECK-NEXT: } + // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 + // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 + // CHECK-NEXT: sv.if [[AND]] { + // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 + // CHECK-NEXT: sv.fwrite %[[STDERR]], "Hexadecimal: %x %0x %4x\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5 + // CHECK-NEXT: } + // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 + // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 + // CHECK-NEXT: sv.if [[AND]] { + // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 + // CHECK-NEXT: sv.fwrite %[[STDERR]], "ASCII Character: %c\0A"([[ADD]]) : i5 + // CHECK-NEXT: } + // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 + // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 + // CHECK-NEXT: sv.if [[AND]] { + // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 + // CHECK-NEXT: [[SUMSIGNED:%.+]] = sv.system "signed"([[ADDSIGNED]]) + // CHECK-NEXT: [[DSIGNED:%.+]] = sv.system "signed"(%d) + // CHECK-NEXT: sv.fwrite %[[STDERR]], "Hi signed %d %d\0A"([[SUMSIGNED]], [[DSIGNED]]) : i5, i4 + // CHECK-NEXT: } + // CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 + // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 + // CHECK-NEXT: sv.if [[AND]] { + // CHECK-NEXT: %[[STDERR:.+]] = hw.constant -2147483646 : i32 + // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 + // CHECK-NEXT: sv.fwrite %[[STDERR]], "[%0t]: %d %m"([[TIME]], %a) : i64, i4 + // CHECK-NEXT: } + // CHECK-NEXT: sv.if %reset { + // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 + // CHECK-NEXT: [[STR:%.+]] = sv.sformatf "%0t%d.txt"([[TIME]], %a) : i64, i4 + // CHECK-NEXT: [[FD:%.+]] = sv.func.call.procedural @"__circt_lib_logging::FileDescriptor::get"([[STR]]) : (!hw.string) -> i32 + // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 + // CHECK-NEXT: sv.fwrite [[FD]], "[%0t]: dynamic file name\0A"([[TIME]]) : i64 + // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 + // CHECK-NEXT: [[STR:%.+]] = sv.sformatf "%0t%d.txt"([[TIME]], %a) : i64, i4 + // CHECK-NEXT: [[FD:%.+]] = sv.func.call.procedural @"__circt_lib_logging::FileDescriptor::get"([[STR]]) : (!hw.string) -> i32 + // CHECK-NEXT: sv.fflush fd [[FD]] + // CHECK-NEXT: } firrtl.printf %clock, %reset, "No operands and literal: %%\0A" : !firrtl.clock, !firrtl.uint<1> %0 = firrtl.add %a, %a : (!firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<5> diff --git a/test/firtool/lower-layers.fir b/test/firtool/lower-layers.fir index e4e4c8823cd2..b4564bcbcb19 100644 --- a/test/firtool/lower-layers.fir +++ b/test/firtool/lower-layers.fir @@ -127,10 +127,10 @@ circuit TestHarness: define trace = x ; CHECK: module TestHarness_Verification() - ; CHECK: always @(posedge TestHarness.clock) begin - ; CHECK: if ((`PRINTF_COND_) & TestHarness.reset) - ; CHECK: $fwrite(32'h80000002, "The last PC was: %x", TestHarness.dut.verification.pc_d); - ; CHECK: end // always @(posedge) + ; CHECK: always @(posedge TestHarness.clock) begin + ; CHECK: if ((`PRINTF_COND_) & TestHarness.reset) + ; CHECK: $fwrite(32'h80000002, "The last PC was: %x", TestHarness.dut.verification.pc_d); + ; CHECK: end // always @(posedge) ; CHECK: endmodule ; CHECK: module TestHarness( From 43676405f555e77f918b12ac128c34b5bdd918b8 Mon Sep 17 00:00:00 2001 From: woshiren Date: Fri, 29 May 2026 13:53:21 +0800 Subject: [PATCH 3/7] use resolveMacroSymName --- include/circt/Dialect/SV/SVOps.h | 9 +++ lib/Conversion/Utils/SVLoweringUtils.cpp | 37 ++++++++---- lib/Dialect/SV/SVOps.cpp | 31 ++++++++++ .../SV/Transforms/SVMaskNonSynthesizable.cpp | 24 -------- unittests/Conversion/CMakeLists.txt | 2 + unittests/Conversion/Utils/CMakeLists.txt | 11 ++++ .../Conversion/Utils/SVLoweringUtilsTest.cpp | 58 +++++++++++++++++++ 7 files changed, 137 insertions(+), 35 deletions(-) create mode 100644 unittests/Conversion/Utils/CMakeLists.txt create mode 100644 unittests/Conversion/Utils/SVLoweringUtilsTest.cpp diff --git a/include/circt/Dialect/SV/SVOps.h b/include/circt/Dialect/SV/SVOps.h index b629263a4864..3946e5890442 100644 --- a/include/circt/Dialect/SV/SVOps.h +++ b/include/circt/Dialect/SV/SVOps.h @@ -42,6 +42,15 @@ bool isExpression(Operation *op); /// Returns if the expression is known to be 2-state (binary) bool is2StateExpression(Value v); +/// Resolve the symbol name to use for the `sv.macro.decl` referenced by macro +/// users. Returns the existing decl's sym_name if a `sv.macro.decl` whose +/// Verilog identifier matches `verilogName` already exists in `symbolTableOp`; +/// otherwise returns a fresh sym_name that does not collide with any existing +/// symbol in `symbolTableOp`. `created` is set to true iff the returned name +/// belongs to a decl that the caller still needs to materialize. +StringAttr resolveMacroSymName(Operation *symbolTableOp, StringRef verilogName, + bool &created); + //===----------------------------------------------------------------------===// // CaseOp Support //===----------------------------------------------------------------------===// diff --git a/lib/Conversion/Utils/SVLoweringUtils.cpp b/lib/Conversion/Utils/SVLoweringUtils.cpp index 0fdb403e1ef5..6f414a41d4fa 100644 --- a/lib/Conversion/Utils/SVLoweringUtils.cpp +++ b/lib/Conversion/Utils/SVLoweringUtils.cpp @@ -62,13 +62,16 @@ void sv::emitFileDescriptorRuntime(Operation *fileScopeOp, auto getterSymName = ::getFileDescriptorGetterSymName(builder.getContext()); auto fragmentSymName = ::getFileDescriptorFragmentSymName(builder.getContext()); - auto macroSymName = builder.getStringAttr(kFileDescriptorMacroName); - auto synthesisSymName = builder.getStringAttr("SYNTHESIS"); + bool fileDescriptorMacroNeedsCreation = false; + auto macroSymName = sv::resolveMacroSymName( + fileScopeOp, kFileDescriptorMacroName, fileDescriptorMacroNeedsCreation); + bool synthesisMacroNeedsCreation = false; + auto synthesisSymName = sv::resolveMacroSymName(fileScopeOp, "SYNTHESIS", + synthesisMacroNeedsCreation); SymbolTable symbolTable(fileScopeOp); - auto emitGuard = [&](StringRef guard, llvm::function_ref body) { - sv::IfDefOp::create( - builder, guard, [] {}, body); + auto emitGuard = [&](StringAttr guard, llvm::function_ref body) { + sv::IfDefOp::create(builder, guard, [] {}, body); }; if (!symbolTable.lookup(getterSymName)) { @@ -96,18 +99,30 @@ void sv::emitFileDescriptorRuntime(Operation *fileScopeOp, symbolTable.insert(func); } - if (!symbolTable.lookup(synthesisSymName)) - symbolTable.insert(sv::MacroDeclOp::create(builder, synthesisSymName)); + if (synthesisMacroNeedsCreation) { + auto verilogName = synthesisSymName.getValue() == "SYNTHESIS" + ? StringAttr{} + : builder.getStringAttr("SYNTHESIS"); + symbolTable.insert( + sv::MacroDeclOp::create(builder, builder.getLoc(), synthesisSymName, + /*args=*/ArrayAttr{}, verilogName)); + } - if (!symbolTable.lookup(macroSymName)) - symbolTable.insert(sv::MacroDeclOp::create(builder, macroSymName)); + if (fileDescriptorMacroNeedsCreation) { + auto verilogName = macroSymName.getValue() == kFileDescriptorMacroName + ? StringAttr{} + : builder.getStringAttr(kFileDescriptorMacroName); + symbolTable.insert( + sv::MacroDeclOp::create(builder, builder.getLoc(), macroSymName, + /*args=*/ArrayAttr{}, verilogName)); + } if (symbolTable.lookup(fragmentSymName)) return; auto fragment = emit::FragmentOp::create(builder, fragmentSymName, [&] { - emitGuard("SYNTHESIS", [&]() { - emitGuard(kFileDescriptorMacroName, [&]() { + emitGuard(synthesisSymName, [&]() { + emitGuard(macroSymName, [&]() { sv::VerbatimOp::create(builder, R"(// CIRCT Logging Library package __circt_lib_logging; class FileDescriptor; diff --git a/lib/Dialect/SV/SVOps.cpp b/lib/Dialect/SV/SVOps.cpp index c48d14cc8ccc..c35130d39eca 100644 --- a/lib/Dialect/SV/SVOps.cpp +++ b/lib/Dialect/SV/SVOps.cpp @@ -21,6 +21,7 @@ #include "circt/Dialect/HW/ModuleImplementation.h" #include "circt/Dialect/SV/SVAttributes.h" #include "circt/Support/CustomDirectiveImpl.h" +#include "circt/Support/Namespace.h" #include "circt/Support/ProceduralRegionTrait.h" #include "mlir/IR/Builders.h" #include "mlir/IR/BuiltinTypes.h" @@ -56,6 +57,36 @@ bool sv::isExpression(Operation *op) { MacroRefExprOp, MacroRefExprSEOp>(op); } +StringAttr sv::resolveMacroSymName(Operation *symbolTableOp, + StringRef verilogName, bool &created) { + assert(symbolTableOp && "expected symbol table op"); + assert(symbolTableOp->hasTrait() && + "expected symbolTableOp to define a symbol table"); + assert(symbolTableOp->getNumRegions() == 1 && + "expected single-region symbol table op"); + assert(!symbolTableOp->getRegion(0).empty() && + "expected non-empty symbol table region"); + + for (auto decl : + symbolTableOp->getRegion(0).front().getOps()) { + if (decl.getMacroIdentifier() == verilogName) { + created = false; + return decl.getSymNameAttr(); + } + } + + Namespace ns; + auto symbolNameId = StringAttr::get(symbolTableOp->getContext(), + SymbolTable::getSymbolAttrName()); + for (auto &op : symbolTableOp->getRegion(0).front()) { + if (auto symbol = op.getAttrOfType(symbolNameId)) + ns.add(symbol.getValue()); + } + + created = true; + return StringAttr::get(symbolTableOp->getContext(), ns.newName(verilogName)); +} + /// Returns the operation registered with the given symbol name with the regions /// of 'symbolTableOp'. recurse through nested regions which don't contain the /// symboltable trait. Returns nullptr if no valid symbol was found. diff --git a/lib/Dialect/SV/Transforms/SVMaskNonSynthesizable.cpp b/lib/Dialect/SV/Transforms/SVMaskNonSynthesizable.cpp index d2df884735ed..200c098d848e 100644 --- a/lib/Dialect/SV/Transforms/SVMaskNonSynthesizable.cpp +++ b/lib/Dialect/SV/Transforms/SVMaskNonSynthesizable.cpp @@ -22,7 +22,6 @@ #include "circt/Dialect/SV/SVAttributes.h" #include "circt/Dialect/SV/SVOps.h" #include "circt/Dialect/SV/SVPasses.h" -#include "circt/Support/Namespace.h" #include "circt/Support/ProceduralRegionTrait.h" #include "circt/Support/Utils.h" #include "mlir/IR/Builders.h" @@ -64,29 +63,6 @@ static Region *getMatchingIfDefElseRegion(Operation *op, .Default(static_cast(nullptr)); } -/// Resolve the symbol name to use for the `sv.macro.decl` referenced by -/// `ifdef` mode's `sv.ifdef` ops. Returns the existing decl's sym_name if a -/// `sv.macro.decl` whose Verilog identifier matches `verilogName` already -/// exists at top level; otherwise returns a fresh sym_name that does not -/// collide with any existing top-level symbol (which may differ from -/// `verilogName` if a non-`sv.macro.decl` symbol of that name is present). -/// `created` is set to true iff the returned name belongs to a decl that -/// the caller still needs to materialize. -static StringAttr resolveMacroSymName(mlir::ModuleOp moduleOp, - StringRef verilogName, bool &created) { - for (auto decl : moduleOp.getOps()) { - if (decl.getMacroIdentifier() == verilogName) { - created = false; - return decl.getSymNameAttr(); - } - } - Namespace ns; - ns.add(moduleOp); - StringRef name = ns.newName(verilogName); - created = true; - return StringAttr::get(moduleOp.getContext(), name); -} - /// `delete` mode: walk the block and erase every masked op. static void processBlockDelete(Block &block) { block.walk([&](Operation *op) { diff --git a/unittests/Conversion/CMakeLists.txt b/unittests/Conversion/CMakeLists.txt index c68eda498977..4cc21dd52092 100644 --- a/unittests/Conversion/CMakeLists.txt +++ b/unittests/Conversion/CMakeLists.txt @@ -1,3 +1,5 @@ +add_subdirectory(Utils) + if(CIRCT_SLANG_FRONTEND_ENABLED) add_subdirectory(ImportVerilog) endif() diff --git a/unittests/Conversion/Utils/CMakeLists.txt b/unittests/Conversion/Utils/CMakeLists.txt new file mode 100644 index 000000000000..190440377360 --- /dev/null +++ b/unittests/Conversion/Utils/CMakeLists.txt @@ -0,0 +1,11 @@ +add_circt_unittest(CIRCTConversionUtilsTests + SVLoweringUtilsTest.cpp +) + +target_link_libraries(CIRCTConversionUtilsTests + PRIVATE + CIRCTEmit + CIRCTHW + CIRCTSV + CIRCTSVLoweringUtils +) diff --git a/unittests/Conversion/Utils/SVLoweringUtilsTest.cpp b/unittests/Conversion/Utils/SVLoweringUtilsTest.cpp new file mode 100644 index 000000000000..8977de792d83 --- /dev/null +++ b/unittests/Conversion/Utils/SVLoweringUtilsTest.cpp @@ -0,0 +1,58 @@ +//===- SVLoweringUtilsTest.cpp - SV lowering utility tests ----------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "circt/Conversion/SVLoweringUtils.h" +#include "circt/Dialect/Emit/EmitDialect.h" +#include "circt/Dialect/Emit/EmitOps.h" +#include "circt/Dialect/HW/HWDialect.h" +#include "circt/Dialect/HW/HWOps.h" +#include "circt/Dialect/SV/SVDialect.h" +#include "circt/Dialect/SV/SVOps.h" +#include "mlir/IR/ImplicitLocOpBuilder.h" +#include "gtest/gtest.h" + +using namespace circt; +using namespace mlir; + +namespace { + +class SVLoweringUtilsTest : public ::testing::Test { +protected: + SVLoweringUtilsTest() { + context.loadDialect(); + loc = UnknownLoc::get(&context); + module = ModuleOp::create(loc); + builder = std::make_unique( + ImplicitLocOpBuilder::atBlockEnd(loc, module.getBody())); + } + + MLIRContext context; + LocationAttr loc; + ModuleOp module; + std::unique_ptr builder; +}; + +TEST_F(SVLoweringUtilsTest, FileDescriptorRuntimeUsesResolvedMacroSymbol) { + hw::HWModuleOp::create(*builder, builder->getStringAttr("SYNTHESIS"), + ArrayRef{}); + + sv::emitFileDescriptorRuntime(module, *builder); + + auto synthesisMacro = module.lookupSymbol("SYNTHESIS_0"); + ASSERT_TRUE(synthesisMacro); + EXPECT_EQ(synthesisMacro.getMacroIdentifier(), "SYNTHESIS"); + + auto fragment = + module.lookupSymbol("CIRCT_LIB_LOGGING_FRAGMENT"); + ASSERT_TRUE(fragment); + auto ifdef = dyn_cast(&fragment.getBody()->front()); + ASSERT_TRUE(ifdef); + EXPECT_EQ(ifdef.getCond().getName(), "SYNTHESIS_0"); +} + +} // namespace From e760eda7514afd916119f3c36a307a54c1bbd92a Mon Sep 17 00:00:00 2001 From: woshiren Date: Fri, 29 May 2026 13:56:10 +0800 Subject: [PATCH 4/7] fix format --- lib/Conversion/Utils/SVLoweringUtils.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Conversion/Utils/SVLoweringUtils.cpp b/lib/Conversion/Utils/SVLoweringUtils.cpp index 6f414a41d4fa..b8f250ffbdea 100644 --- a/lib/Conversion/Utils/SVLoweringUtils.cpp +++ b/lib/Conversion/Utils/SVLoweringUtils.cpp @@ -71,7 +71,8 @@ void sv::emitFileDescriptorRuntime(Operation *fileScopeOp, SymbolTable symbolTable(fileScopeOp); auto emitGuard = [&](StringAttr guard, llvm::function_ref body) { - sv::IfDefOp::create(builder, guard, [] {}, body); + sv::IfDefOp::create( + builder, guard, [] {}, body); }; if (!symbolTable.lookup(getterSymName)) { From f587125e5952b0ffe6b090689693531d6aafc8e5 Mon Sep 17 00:00:00 2001 From: woshiren Date: Fri, 29 May 2026 14:14:23 +0800 Subject: [PATCH 5/7] Trigger build From 97326ff0148c1ced2d2d0e899edebd74ea205662 Mon Sep 17 00:00:00 2001 From: woshiren Date: Sat, 30 May 2026 14:50:10 +0800 Subject: [PATCH 6/7] fix nits --- include/circt/Dialect/SV/SVOps.h | 17 ++++---- lib/Conversion/Utils/SVLoweringUtils.cpp | 43 +++++++++---------- lib/Dialect/SV/SVOps.cpp | 26 +++++------ .../SV/Transforms/SVMaskNonSynthesizable.cpp | 3 +- 4 files changed, 41 insertions(+), 48 deletions(-) diff --git a/include/circt/Dialect/SV/SVOps.h b/include/circt/Dialect/SV/SVOps.h index 3946e5890442..6d8f61c93cdd 100644 --- a/include/circt/Dialect/SV/SVOps.h +++ b/include/circt/Dialect/SV/SVOps.h @@ -42,14 +42,15 @@ bool isExpression(Operation *op); /// Returns if the expression is known to be 2-state (binary) bool is2StateExpression(Value v); -/// Resolve the symbol name to use for the `sv.macro.decl` referenced by macro -/// users. Returns the existing decl's sym_name if a `sv.macro.decl` whose -/// Verilog identifier matches `verilogName` already exists in `symbolTableOp`; -/// otherwise returns a fresh sym_name that does not collide with any existing -/// symbol in `symbolTableOp`. `created` is set to true iff the returned name -/// belongs to a decl that the caller still needs to materialize. -StringAttr resolveMacroSymName(Operation *symbolTableOp, StringRef verilogName, - bool &created); +/// Look up or generate the symbol name to use for the `sv.macro.decl` +/// referenced by macro users. Returns the existing decl's sym_name if a +/// `sv.macro.decl` whose Verilog identifier matches `verilogName` already +/// exists in `symbolTableBlock`; otherwise returns a fresh sym_name that does +/// not collide with any existing symbol in `symbolTableBlock`. `created` is set +/// to true iff the returned name belongs to a decl that the caller still needs +/// to materialize. +StringAttr lookupOrGenerateMacroSymName(Block *symbolTableBlock, + StringRef verilogName, bool &created); //===----------------------------------------------------------------------===// // CaseOp Support diff --git a/lib/Conversion/Utils/SVLoweringUtils.cpp b/lib/Conversion/Utils/SVLoweringUtils.cpp index b8f250ffbdea..6a95f1e3ded9 100644 --- a/lib/Conversion/Utils/SVLoweringUtils.cpp +++ b/lib/Conversion/Utils/SVLoweringUtils.cpp @@ -62,17 +62,18 @@ void sv::emitFileDescriptorRuntime(Operation *fileScopeOp, auto getterSymName = ::getFileDescriptorGetterSymName(builder.getContext()); auto fragmentSymName = ::getFileDescriptorFragmentSymName(builder.getContext()); + auto *fileScopeBlock = &fileScopeOp->getRegion(0).front(); bool fileDescriptorMacroNeedsCreation = false; - auto macroSymName = sv::resolveMacroSymName( - fileScopeOp, kFileDescriptorMacroName, fileDescriptorMacroNeedsCreation); + auto macroSymName = + sv::lookupOrGenerateMacroSymName(fileScopeBlock, kFileDescriptorMacroName, + fileDescriptorMacroNeedsCreation); bool synthesisMacroNeedsCreation = false; - auto synthesisSymName = sv::resolveMacroSymName(fileScopeOp, "SYNTHESIS", - synthesisMacroNeedsCreation); + auto synthesisSymName = sv::lookupOrGenerateMacroSymName( + fileScopeBlock, "SYNTHESIS", synthesisMacroNeedsCreation); SymbolTable symbolTable(fileScopeOp); auto emitGuard = [&](StringAttr guard, llvm::function_ref body) { - sv::IfDefOp::create( - builder, guard, [] {}, body); + sv::IfDefOp::create(builder, guard, [] {}, body); }; if (!symbolTable.lookup(getterSymName)) { @@ -100,23 +101,19 @@ void sv::emitFileDescriptorRuntime(Operation *fileScopeOp, symbolTable.insert(func); } - if (synthesisMacroNeedsCreation) { - auto verilogName = synthesisSymName.getValue() == "SYNTHESIS" - ? StringAttr{} - : builder.getStringAttr("SYNTHESIS"); - symbolTable.insert( - sv::MacroDeclOp::create(builder, builder.getLoc(), synthesisSymName, - /*args=*/ArrayAttr{}, verilogName)); - } - - if (fileDescriptorMacroNeedsCreation) { - auto verilogName = macroSymName.getValue() == kFileDescriptorMacroName - ? StringAttr{} - : builder.getStringAttr(kFileDescriptorMacroName); - symbolTable.insert( - sv::MacroDeclOp::create(builder, builder.getLoc(), macroSymName, - /*args=*/ArrayAttr{}, verilogName)); - } + if (synthesisMacroNeedsCreation) + symbolTable.insert(sv::MacroDeclOp::create( + builder, builder.getLoc(), synthesisSymName, /*args=*/ArrayAttr{}, + /*verilogName=*/synthesisSymName.getValue() == "SYNTHESIS" + ? StringAttr{} + : builder.getStringAttr("SYNTHESIS"))); + + if (fileDescriptorMacroNeedsCreation) + symbolTable.insert(sv::MacroDeclOp::create( + builder, builder.getLoc(), macroSymName, /*args=*/ArrayAttr{}, + /*verilogName=*/macroSymName.getValue() == kFileDescriptorMacroName + ? StringAttr{} + : builder.getStringAttr(kFileDescriptorMacroName))); if (symbolTable.lookup(fragmentSymName)) return; diff --git a/lib/Dialect/SV/SVOps.cpp b/lib/Dialect/SV/SVOps.cpp index c35130d39eca..70c48a74355c 100644 --- a/lib/Dialect/SV/SVOps.cpp +++ b/lib/Dialect/SV/SVOps.cpp @@ -57,18 +57,10 @@ bool sv::isExpression(Operation *op) { MacroRefExprOp, MacroRefExprSEOp>(op); } -StringAttr sv::resolveMacroSymName(Operation *symbolTableOp, - StringRef verilogName, bool &created) { - assert(symbolTableOp && "expected symbol table op"); - assert(symbolTableOp->hasTrait() && - "expected symbolTableOp to define a symbol table"); - assert(symbolTableOp->getNumRegions() == 1 && - "expected single-region symbol table op"); - assert(!symbolTableOp->getRegion(0).empty() && - "expected non-empty symbol table region"); - - for (auto decl : - symbolTableOp->getRegion(0).front().getOps()) { +StringAttr sv::lookupOrGenerateMacroSymName(Block *symbolTableBlock, + StringRef verilogName, + bool &created) { + for (auto decl : symbolTableBlock->getOps()) { if (decl.getMacroIdentifier() == verilogName) { created = false; return decl.getSymNameAttr(); @@ -76,15 +68,17 @@ StringAttr sv::resolveMacroSymName(Operation *symbolTableOp, } Namespace ns; - auto symbolNameId = StringAttr::get(symbolTableOp->getContext(), - SymbolTable::getSymbolAttrName()); - for (auto &op : symbolTableOp->getRegion(0).front()) { + auto symbolNameId = + StringAttr::get(symbolTableBlock->getParentOp()->getContext(), + SymbolTable::getSymbolAttrName()); + for (auto &op : *symbolTableBlock) { if (auto symbol = op.getAttrOfType(symbolNameId)) ns.add(symbol.getValue()); } created = true; - return StringAttr::get(symbolTableOp->getContext(), ns.newName(verilogName)); + return StringAttr::get(symbolTableBlock->getParentOp()->getContext(), + ns.newName(verilogName)); } /// Returns the operation registered with the given symbol name with the regions diff --git a/lib/Dialect/SV/Transforms/SVMaskNonSynthesizable.cpp b/lib/Dialect/SV/Transforms/SVMaskNonSynthesizable.cpp index 200c098d848e..0a32956b6bdb 100644 --- a/lib/Dialect/SV/Transforms/SVMaskNonSynthesizable.cpp +++ b/lib/Dialect/SV/Transforms/SVMaskNonSynthesizable.cpp @@ -140,7 +140,8 @@ void SVMaskNonSynthesizablePass::runOnOperation() { StringAttr macroSymName; bool macroDeclNeedsCreation = false; if (mode == MaskNonSynthesizableMode::Ifdef) - macroSymName = resolveMacroSymName(moduleOp, macro, macroDeclNeedsCreation); + macroSymName = lookupOrGenerateMacroSymName(moduleOp.getBody(), macro, + macroDeclNeedsCreation); StringRef passDownMacro = macroSymName ? macroSymName.getValue() : StringRef(); From 92cd9c269d22447bd9e8aa0fdccd2d65bb1fcc09 Mon Sep 17 00:00:00 2001 From: woshiren Date: Sat, 30 May 2026 14:52:49 +0800 Subject: [PATCH 7/7] fix format --- lib/Conversion/Utils/SVLoweringUtils.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Conversion/Utils/SVLoweringUtils.cpp b/lib/Conversion/Utils/SVLoweringUtils.cpp index 6a95f1e3ded9..dd01bc87e4be 100644 --- a/lib/Conversion/Utils/SVLoweringUtils.cpp +++ b/lib/Conversion/Utils/SVLoweringUtils.cpp @@ -73,7 +73,8 @@ void sv::emitFileDescriptorRuntime(Operation *fileScopeOp, SymbolTable symbolTable(fileScopeOp); auto emitGuard = [&](StringAttr guard, llvm::function_ref body) { - sv::IfDefOp::create(builder, guard, [] {}, body); + sv::IfDefOp::create( + builder, guard, [] {}, body); }; if (!symbolTable.lookup(getterSymName)) {