From 9f7ae1e82adc499f232065a6e0918786e01638ab Mon Sep 17 00:00:00 2001 From: Hideto Ueno Date: Fri, 18 Apr 2025 17:34:43 -0700 Subject: [PATCH] [LowerToHW] [ExtractTestCode] Remove FD caching to pass ETC (#8428) Using initilaization for lowering file descriptor casuses probems for ETC so this commit removes initialization and call library function every time fd is necessary --- lib/Conversion/FIRRTLToHW/LowerToHW.cpp | 63 ++----------------- .../SV/Transforms/SVExtractTestCode.cpp | 6 +- test/Conversion/FIRRTLToHW/lower-to-hw.mlir | 57 ++++++----------- test/Dialect/SV/hw-extract-test-code.mlir | 8 +++ test/firtool/print.fir | 44 +++++-------- 5 files changed, 52 insertions(+), 126 deletions(-) diff --git a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp index b75fcc960f..a4a0b9682f 100644 --- a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp +++ b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp @@ -209,14 +209,6 @@ public: assert(outputFileName || substitutions.empty() && "substitutions must be empty when output file name is empty"); - isDynamicFileName = llvm::any_of(substitutions, [](Value v) { - auto *op = v.getDefiningOp(); - if (!op) - return true; - // HierarchicalModuleNameOp is a constant. - return !(isa(op) || - op->hasTrait()); - }); } FileDescriptorInfo() = default; @@ -229,13 +221,8 @@ public: StringAttr getOutputFileFormat() const { return outputFileFormat; } mlir::ValueRange getSubstitutions() const { return substitutions; } - bool isDynamicOutputFileName() const { return isDynamicFileName; } private: - // Set true if the file name is dynamic, i.e. $time or normal hardware value - // is substituted. - bool isDynamicFileName; - // "Verilog" format string for the output file. StringAttr outputFileFormat = {}; @@ -2652,34 +2639,6 @@ LogicalResult FIRRTLLowering::lowerStatementWithFd( bool failed = false; circuitState.addMacroDecl(builder.getStringAttr("SYNTHESIS")); addToIfDefBlock("SYNTHESIS", std::function(), [&]() { - // If the file descriptor is a file, initilize a file descriptor from a - // static class function. - if (!fileDescriptor.isDefaultFd() && - !fileDescriptor.isDynamicOutputFileName()) { - auto outputFile = fileDescriptor.getOutputFileFormat(); - // `ifndef SYNTHESIS - // reg fd_; - // initial begin - // fd_ = $fopen(")); - // end - // `endif - auto &fd = fileNameToFileDescriptor[outputFile]; - if (!fd) { - fd = builder.create( - builder.getIntegerType(32), - builder.getStringAttr("fd_" + outputFile.getValue())); - - addToInitialBlock([&]() { - auto fdOrError = callFileDescriptorLib(fileDescriptor); - if (llvm::failed(fdOrError)) { - failed = true; - return; - } - builder.create(fd, *fdOrError); - }); - } - } - 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. @@ -2700,23 +2659,13 @@ LogicalResult FIRRTLLowering::lowerStatementWithFd( "PRINTF_FD_"); circuitState.addFragment(theModule, "PRINTF_FD_FRAGMENT"); } else { - if (fileDescriptor.isDynamicOutputFileName()) { - // If the file name is dynamic, call the library function to get the - // FD. - auto fdOrError = callFileDescriptorLib(fileDescriptor); - if (llvm::failed(fdOrError)) { - failed = true; - return; - } - fd = *fdOrError; - } else { - // Otherwise, read the FD from the register. - auto it = fileNameToFileDescriptor.find( - fileDescriptor.getOutputFileFormat()); - assert(it != fileNameToFileDescriptor.end() && - "must be registered"); - fd = builder.create(it->second); + // Call the library function to get the FD. + auto fdOrError = callFileDescriptorLib(fileDescriptor); + if (llvm::failed(fdOrError)) { + failed = true; + return; } + fd = *fdOrError; } failed = llvm::failed(fn(fd)); }); diff --git a/lib/Dialect/SV/Transforms/SVExtractTestCode.cpp b/lib/Dialect/SV/Transforms/SVExtractTestCode.cpp index 91baa9800e..1387ad8ab7 100644 --- a/lib/Dialect/SV/Transforms/SVExtractTestCode.cpp +++ b/lib/Dialect/SV/Transforms/SVExtractTestCode.cpp @@ -562,8 +562,8 @@ static bool isAssertOp(hw::HWSymbolCache &symCache, Operation *op) { return false; } - return isa(op); + return isa(op); } static bool isCoverOp(hw::HWSymbolCache &symCache, Operation *op) { @@ -628,7 +628,7 @@ bool isInDesign(hw::HWSymbolCache &symCache, Operation *op, return false; // Special case some operations which we want to clone. - if (isa(op)) + if (isa(op)) return false; // Otherwise, operations with memory effects as a part design. diff --git a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir index 4630b0b1d8..175a7518e5 100644 --- a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir +++ b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir @@ -361,16 +361,6 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // CHECK: sv.ifdef @SYNTHESIS { // CHECK-NEXT: } else { - // CHECK-NEXT: %[[FD_0:.+]] = sv.reg : !hw.inout - // CHECK-NEXT: %[[FD_1:.+]] = sv.reg name "fd_%m%c.txt" : !hw.inout - // CHECK-NEXT: sv.initial { - // CHECK-NEXT: %[[FILE_NAME:.+]] = sv.constantStr "file.txt" - // CHECK-NEXT: %[[FD_RESULT:.+]] = sv.func.call.procedural @"__circt_lib_logging::FileDescriptor::get"(%[[FILE_NAME]]) : (!hw.string) -> i32 - // CHECK-NEXT: sv.bpassign %[[FD_0]], %[[FD_RESULT]] : i32 - // CHECK-NEXT: %[[FILE_NAME:.+]] = sv.sformatf "%m%c.txt"(%c97_i8) - // CHECK-NEXT: %[[FD_RESULT:.+]] = sv.func.call.procedural @"__circt_lib_logging::FileDescriptor::get"(%[[FILE_NAME]]) : (!hw.string) -> i32 - // CHECK-NEXT: sv.bpassign %[[FD_1]], %[[FD_RESULT]] : i32 - // CHECK-NEXT: } // CHECK-NEXT: sv.always posedge [[CLOCK]] { // CHECK-NEXT: %[[PRINTF_COND:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 // CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND]], %reset @@ -417,31 +407,23 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 // CHECK-NEXT: sv.fwrite %PRINTF_FD_, "[%0t]: %d %m"([[TIME]], %a) : i64, 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: %[[FPRINTF_FD_:.+]] = sv.read_inout %fd_file.txt : !hw.inout - // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 - // CHECK-NEXT: sv.fwrite %[[FPRINTF_FD_]], "[%0t]: %d %m"([[TIME]], %a) : i64, i4 - // CHECK-NEXT: } - // CHECK-NEXT: [[COND:%.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 - // CHECK-NEXT: [[AND:%.+]] = comb.and bin [[COND]], %reset : i1 - // CHECK-NEXT: sv.if [[AND]] { - // CHECK-NEXT: [[FD_STATIC:%.+]] = sv.read_inout %[[FD_1]] : !hw.inout - // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 - // CHECK-NEXT: sv.fwrite [[FD_STATIC]], "[%0t]: static file name (w/ substitution)\0A"([[TIME]]) : i64 - // CHECK-NEXT: } - // CHECK-NEXT: [[COND:%.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 - // CHECK-NEXT: [[AND:%.+]] = comb.and bin [[COND]], %reset : i1 - // CHECK-NEXT: sv.if [[AND]] { - // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 - // CHECK-NEXT: [[FNAME:%.+]] = sv.sformatf "%0t%d.txt"([[TIME]], %a) : i64, i4 - // CHECK-NEXT: [[FD_DYN:%.+]] = sv.func.call.procedural @"__circt_lib_logging::FileDescriptor::get"([[FNAME]]) : (!hw.string) -> i32 - // CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64 - // CHECK-NEXT: sv.fwrite [[FD_DYN]], "[%0t]: dynamic file name\0A"([[TIME]]) : i64 - // CHECK-NEXT: } - // CHECK-NEXT: } - // CHECK-NEXT: } + // CEHCK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 + // CEHCK-NEXT: %[[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 + // CEHCK-NEXT: sv.if %[[AND]] { + // CEHCK-NEXT: [[TIME:%.+]] = sv.system.time : i64 + // CEHCK-NEXT: [[STR:%.+]] = sv.sformatf "%0t%d.txt"(%[[TIME]], %a) : i64, i4 + // CEHCK-NEXT: [[FD:%.+]] = sv.func.call.procedural @"__circt_lib_logging::FileDescriptor::get"(%[[STR]]) : (!hw.string) -> i32 + // CEHCK-NEXT: [[TIME:%.+]] = sv.system.time : i64 + // CEHCK-NEXT: sv.fwrite %[[FD]], "[%0t]: dynamic file name\0A"(%[[TIME]]) : i64 + // CEHCK-NEXT: } + // CEHCK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1 + // CEHCK-NEXT: %[[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1 + // CEHCK-NEXT: sv.if %[[AND]] { + // CEHCK-NEXT: [[TIME:%.+]] = sv.system.time : i64 + // CEHCK-NEXT: [[STR:%.+]] = sv.sformatf "%0t%d.txt"(%[[TIME]], %a) : i64, i4 + // CEHCK-NEXT: [[FD:%.+]] = sv.func.call.procedural @"__circt_lib_logging::FileDescriptor::get"(%[[STR]]) : (!hw.string) -> i32 + // CEHCK-NEXT: sv.fflush fd %[[FD]] + // CEHCK-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> @@ -462,10 +444,9 @@ firrtl.circuit "Simple" attributes {annotations = [{class = %hierarchicalmodulename = firrtl.fstring.hierarchicalmodulename : !firrtl.fstring firrtl.printf %clock, %reset, "[{{}}]: %d {{}}" (%time, %a, %hierarchicalmodulename) : !firrtl.clock, !firrtl.uint<1>, !firrtl.fstring, !firrtl.uint<4>, !firrtl.fstring - firrtl.fprintf %clock, %reset, "file.txt", "[{{}}]: %d {{}}" (%time, %a, %hierarchicalmodulename) : !firrtl.clock, !firrtl.uint<1>, !firrtl.fstring, !firrtl.uint<4>, !firrtl.fstring - %c97_ui8 = firrtl.constant 97 : !firrtl.uint<8> - firrtl.fprintf %clock, %reset, "{{}}%c.txt"(%hierarchicalmodulename, %c97_ui8), "[{{}}]: static file name (w/ substitution)\0A"(%time) : !firrtl.clock, !firrtl.uint<1>, !firrtl.fstring, !firrtl.uint<8>, !firrtl.fstring firrtl.fprintf %clock, %reset, "{{}}%d.txt"(%time, %a), "[{{}}]: dynamic file name\0A"(%time) : !firrtl.clock, !firrtl.uint<1>, !firrtl.fstring, !firrtl.uint<4>, !firrtl.fstring + firrtl.fflush %clock, %reset, "{{}}%d.txt"(%time, %a) : !firrtl.clock, !firrtl.uint<1>, !firrtl.fstring, !firrtl.uint<4> + firrtl.skip // CHECK: hw.output diff --git a/test/Dialect/SV/hw-extract-test-code.mlir b/test/Dialect/SV/hw-extract-test-code.mlir index 5ce287982f..93367b901b 100644 --- a/test/Dialect/SV/hw-extract-test-code.mlir +++ b/test/Dialect/SV/hw-extract-test-code.mlir @@ -16,6 +16,9 @@ // CHECK: sv.error "assert:" // CHECK: sv.error "assertNotX:" // CHECK: sv.error "check [verif-library-assert] is included" +// CHECK: sv.func.call.procedural @fn +// CHECK: sv.fwrite +// CHECK: sv.fflush // CHECK: sv.fatal 1 // CHECK: foo_assert // CHECK: hw.module private @issue1246_assume(in %clock : i1) attributes { @@ -48,6 +51,8 @@ module attributes {firrtl.extract.assert = #hw.output_file<"dir3/", excludeFrom hw.module.extern @foo_cover(in %a : i1) attributes {"firrtl.extract.cover.extra"} hw.module.extern @foo_assume(in %a : i1) attributes {"firrtl.extract.assume.extra"} hw.module.extern @foo_assert(in %a : i1) attributes {"firrtl.extract.assert.extra"} + sv.func private @fn(out out : i32 {sv.func.explicitly_returned}) + hw.module @issue1246(in %clock: i1) attributes {emit.fragments = [@some_fragment]} { sv.always posedge %clock { sv.ifdef.procedural @SYNTHESIS { @@ -58,6 +63,9 @@ module attributes {firrtl.extract.assert = #hw.output_file<"dir3/", excludeFrom sv.error "assert:" sv.error "assertNotX:" sv.error "check [verif-library-assert] is included" + %out = sv.func.call.procedural @fn () : () -> (i32) + sv.fwrite %out, "foo" + sv.fflush fd %out sv.fatal 1 sv.assume %clock, immediate sv.cover %clock, immediate diff --git a/test/firtool/print.fir b/test/firtool/print.fir index ee856ff459..00c2936b4a 100644 --- a/test/firtool/print.fir +++ b/test/firtool/print.fir @@ -1,4 +1,5 @@ ; RUN: firtool %s | FileCheck %s +; RUN: firtool %s --extract-test-code | FileCheck --check-prefix=ETC %s FIRRTL version 5.1.0 ; CHECK: `ifndef __CIRCT_LIB_LOGGING @@ -17,19 +18,15 @@ FIRRTL version 5.1.0 ; CHECK-NEXT: `endif // not def __CIRCT_LIB_LOGGING circuit PrintTest: ; CHECK-LABEL: module PrintTest + ; ETC-LABEL: module PrintTest_assert + ; ETC-LABEL: PrintTest + ; ETC-NOT: $fwrite + ; ETC-NOT: $fflush public module PrintTest : input clock : Clock input cond : UInt<1> input a : UInt<8> - ; CHECK: reg [31:0] [[FD_0:[a-zA-Z0-9_]+]]; - ; CHECK-NEXT: reg [31:0] [[FD_1:[a-zA-Z0-9_]+]]; - ; CHECK-NEXT: initial begin - ; CHECK-NEXT: [[FD_0]] = __circt_lib_logging::FileDescriptor::get("test.txt"); - ; CHECK-NEXT: [[FD_1]] = __circt_lib_logging::FileDescriptor::get($sformatf("%m%c.txt", - ; CHECK-NEXT: 8'h61)); - ; CHECK-NEXT: end - ; CHECK: $fwrite(`PRINTF_FD_, "binary: %b %0b %8b\n", a, a, a); printf(clock, cond, "binary: %b %0b %8b\n", a, a, a) @@ -47,32 +44,23 @@ circuit PrintTest: ; CHECK-NEXT: $fwrite(`PRINTF_FD_, "[%0t]: %m\n", $time); printf(clock, cond, "[{{SimulationTime}}]: {{HierarchicalModuleName}}\n") - - ; CHECK-NEXT: $fwrite([[FD_0]], "hello"); + + ; CHECK-NEXT: ___circt_lib_logging3A3AFileDescriptor3A3Aget_0_1 = __circt_lib_logging::FileDescriptor::get("test.txt"); + ; CHECK-NEXT: $fwrite(___circt_lib_logging3A3AFileDescriptor3A3Aget_0_1, "hello"); fprintf(clock, cond, "test.txt", "hello") - ; CHECK-NEXT: $fwrite([[FD_0]], "[%0t]: %m\n", $time); - fprintf(clock, cond, "test.txt", "[{{SimulationTime}}]: {{HierarchicalModuleName}}\n") - - ; CHECK-NEXT: $fwrite([[FD_0]], "static file name (w/o substitution)\n"); - fprintf(clock, cond, "test.txt", "static file name (w/o substitution)\n") - + ; CHECK-NEXT: ___circt_lib_logging3A3AFileDescriptor3A3Aget_0_0 = __circt_lib_logging::FileDescriptor::get($sformatf("%m%c.txt", + ; CHECK-NEXT: 8'h61)); + ; CHECK-NEXT: $fwrite(___circt_lib_logging3A3AFileDescriptor3A3Aget_0_0, + ; CHECK-NEXT: "[%0t]: static file name (w/ substitution)\n", $time); node c = UInt<8>(97) - ; CHECK-NEXT: $fwrite([[FD_1]], "[%0t]: static file name (w/ substitution)\n", $time); fprintf(clock, cond, "{{HierarchicalModuleName}}%c.txt", c, "[{{SimulationTime}}]: static file name (w/ substitution)\n") - ; CHECK-NEXT: [[FD_2:[a-zA-Z0-9_]+]] = __circt_lib_logging::FileDescriptor::get($sformatf("%0t%d.txt", - ; CHECK-NEXT: $time, - ; CHECK-NEXT: a)); - ; CHECK-NEXT: $fwrite([[FD_2]], - ; CHECK-NEXT: "[%0t]: dynamic file name\n", $time); - fprintf(clock, cond, "{{SimulationTime}}%d.txt", a, "[{{SimulationTime}}]: dynamic file name\n") - ; CHECK-NEXT: $fflush(`PRINTF_FD_); + ; CHECK-NEXT: ___circt_lib_logging3A3AFileDescriptor3A3Aget_0 = __circt_lib_logging::FileDescriptor::get($sformatf("%0t%d.txt", + ; CHECK-NEXT: $time, + ; CHECK-NEXT: a)); fflush(clock, cond) - ; CHECK-NEXT: [[FD_3:[a-zA-Z0-9_]+]] = __circt_lib_logging::FileDescriptor::get($sformatf("%0t%d.txt", - ; CHECK-NEXT: $time, - ; CHECK-NEXT: a)); - ; CHECK-NEXT: $fflush([[FD_3]]); + ; CHECK-NEXT: $fflush(___circt_lib_logging3A3AFileDescriptor3A3Aget_0); fflush(clock, cond, "{{SimulationTime}}%d.txt", a);