From 9fc736bf12f86a148a7778d97d129fcbe5160c1d Mon Sep 17 00:00:00 2001 From: Nandor Licker Date: Mon, 4 Mar 2024 01:37:32 -0800 Subject: [PATCH] [SV] Verify macro reference symbols --- lib/Conversion/SeqToSV/SeqToSV.cpp | 150 ++++++++++++---------- lib/Dialect/SV/SVOps.cpp | 6 +- test/Dialect/SV/hw-extract-test-code.mlir | 4 + test/Dialect/Seq/firreg.mlir | 11 +- test/Dialect/Seq/hw-memsim.mlir | 12 +- test/firtool/firtool.fir | 10 +- 6 files changed, 110 insertions(+), 83 deletions(-) diff --git a/lib/Conversion/SeqToSV/SeqToSV.cpp b/lib/Conversion/SeqToSV/SeqToSV.cpp index 252a9ffee98..f26adbc6320 100644 --- a/lib/Conversion/SeqToSV/SeqToSV.cpp +++ b/lib/Conversion/SeqToSV/SeqToSV.cpp @@ -456,48 +456,69 @@ void SeqToSVPass::runOnOperation() { if (failed(applyPartialConversion(circuit, target, std::move(patterns)))) signalPassFailure(); + bool needsMemRandomization = !memsByModule.empty(); + + auto loc = UnknownLoc::get(context); + auto b = ImplicitLocOpBuilder::atBlockBegin(loc, circuit.getBody()); + if (needsRegRandomization || needsMemRandomization) { + b.create("ENABLE_INITIAL_REG_"); + b.create("ENABLE_INITIAL_MEM_"); + if (needsRegRandomization) { + b.create("FIRRTL_BEFORE_INITIAL"); + b.create("FIRRTL_AFTER_INITIAL"); + } + if (needsMemRandomization) + b.create("RANDOMIZE_MEM_INIT"); + b.create("RANDOMIZE_REG_INIT"); + b.create("RANDOMIZE"); + b.create("RANDOMIZE_DELAY"); + b.create("RANDOM"); + b.create("INIT_RANDOM"); + b.create("INIT_RANDOM_PROLOG_"); + } + bool hasRegRandomization = needsRegRandomization && !disableRegRandomization; - bool hasMemRandomization = !memsByModule.empty() && !disableMemRandomization; + bool hasMemRandomization = needsMemRandomization && !disableMemRandomization; if (!hasRegRandomization && !hasMemRandomization) return; // Build macros for FIRRTL-style register and memory initialization. // Insert them at the start of the module, after any other verbatims. - auto loc = UnknownLoc::get(context); - auto b = ImplicitLocOpBuilder::atBlockBegin(loc, circuit.getBody()); for (Operation &op : *circuit.getBody()) { - if (!isa(&op)) { + if (!isa(&op)) { b.setInsertionPoint(&op); break; } } + // Create SYNTHESIS/VERILATOR macros if other passes have not done so already. + { + StringSet<> symbols; + for (auto sym : circuit.getOps()) + symbols.insert(sym.getName()); + if (!symbols.count("SYNTHESIS")) + b.create("SYNTHESIS"); + if (!symbols.count("VERILATOR")) + b.create("VERILATOR"); + } + // TODO: We could have an operation for macros and uses of them, and // even turn them into symbols so we can DCE unused macro definitions. - StringSet<> emittedDecls; - auto emitDefine = [&](StringRef name, StringRef body, ArrayAttr args = {}) { - if (!emittedDecls.count(name)) { - emittedDecls.insert(name); - OpBuilder::InsertionGuard guard(b); - b.setInsertionPointToStart(circuit.getBody()); - b.create(name, args, StringAttr()); - } - b.create(name, body); - }; auto emitGuardedDefine = [&](StringRef guard, StringRef defName, StringRef defineTrue = "", StringRef defineFalse = StringRef()) { if (!defineFalse.data()) { assert(defineTrue.data() && "didn't define anything"); - b.create(guard, [&]() { emitDefine(defName, defineTrue); }); + b.create( + guard, [&]() { b.create(defName, defineTrue); }); } else { b.create( guard, [&]() { if (defineTrue.data()) - emitDefine(defName, defineTrue); + b.create(defName, defineTrue); }, - [&]() { emitDefine(defName, defineFalse); }); + [&]() { b.create(defName, defineFalse); }); } }; @@ -510,66 +531,57 @@ void SeqToSVPass::runOnOperation() { b.create("// Standard header to adapt well known macros for " "register randomization."); - bool needsRandom = true; - if (hasMemRandomization) { + if (hasMemRandomization) emitGuard("RANDOMIZE", [&]() { emitGuardedDefine("RANDOMIZE_MEM_INIT", "RANDOMIZE"); }); - needsRandom = true; - } - if (hasRegRandomization) { + if (hasRegRandomization) emitGuard("RANDOMIZE", [&]() { emitGuardedDefine("RANDOMIZE_REG_INIT", "RANDOMIZE"); }); - needsRandom = true; - } - if (needsRandom) { - b.create( - "\n// RANDOM may be set to an expression that produces a 32-bit " - "random unsigned value."); - emitGuardedDefine("RANDOM", "RANDOM", StringRef(), "$random"); - - b.create( - "\n// Users can define INIT_RANDOM as general code that gets " - "injected " - "into the\n// initializer block for modules with registers."); - emitGuardedDefine("INIT_RANDOM", "INIT_RANDOM", StringRef(), ""); - - b.create( - "\n// If using random initialization, you can also define " - "RANDOMIZE_DELAY to\n// customize the delay used, otherwise 0.002 " - "is used."); - emitGuardedDefine("RANDOMIZE_DELAY", "RANDOMIZE_DELAY", StringRef(), - "0.002"); - - b.create( - "\n// Define INIT_RANDOM_PROLOG_ for use in our modules below."); - emitGuard("INIT_RANDOM_PROLOG_", [&]() { - b.create( - "RANDOMIZE", - [&]() { - emitGuardedDefine("VERILATOR", "INIT_RANDOM_PROLOG_", - "`INIT_RANDOM", - "`INIT_RANDOM #`RANDOMIZE_DELAY begin end"); - }, - [&]() { emitDefine("INIT_RANDOM_PROLOG_", ""); }); - }); + b.create( + "\n// RANDOM may be set to an expression that produces a 32-bit " + "random unsigned value."); + emitGuardedDefine("RANDOM", "RANDOM", StringRef(), "$random"); + + b.create( + "\n// Users can define INIT_RANDOM as general code that gets " + "injected " + "into the\n// initializer block for modules with registers."); + emitGuardedDefine("INIT_RANDOM", "INIT_RANDOM", StringRef(), ""); + + b.create( + "\n// If using random initialization, you can also define " + "RANDOMIZE_DELAY to\n// customize the delay used, otherwise 0.002 " + "is used."); + emitGuardedDefine("RANDOMIZE_DELAY", "RANDOMIZE_DELAY", StringRef(), "0.002"); + + b.create( + "\n// Define INIT_RANDOM_PROLOG_ for use in our modules below."); + emitGuard("INIT_RANDOM_PROLOG_", [&]() { + b.create( + "RANDOMIZE", + [&]() { + emitGuardedDefine("VERILATOR", "INIT_RANDOM_PROLOG_", "`INIT_RANDOM", + "`INIT_RANDOM #`RANDOMIZE_DELAY begin end"); + }, + [&]() { b.create("INIT_RANDOM_PROLOG_", ""); }); + }); - b.create("\n// Include register initializers in init " - "blocks unless synthesis is set"); - emitGuard("SYNTHESIS", [&] { - emitGuardedDefine("ENABLE_INITIAL_REG_", "ENABLE_INITIAL_REG_", - StringRef(), ""); - }); + b.create("\n// Include register initializers in init " + "blocks unless synthesis is set"); + emitGuard("SYNTHESIS", [&] { + emitGuardedDefine("ENABLE_INITIAL_REG_", "ENABLE_INITIAL_REG_", StringRef(), + ""); + }); - b.create("\n// Include rmemory initializers in init " - "blocks unless synthesis is set"); - emitGuard("SYNTHESIS", [&] { - emitGuardedDefine("ENABLE_INITIAL_MEM_", "ENABLE_INITIAL_MEM_", - StringRef(), ""); - }); - b.create(""); - } + b.create("\n// Include rmemory initializers in init " + "blocks unless synthesis is set"); + emitGuard("SYNTHESIS", [&] { + emitGuardedDefine("ENABLE_INITIAL_MEM_", "ENABLE_INITIAL_MEM_", StringRef(), + ""); + }); + b.create(""); } std::unique_ptr diff --git a/lib/Dialect/SV/SVOps.cpp b/lib/Dialect/SV/SVOps.cpp index a5fc2374a46..39586aef76b 100644 --- a/lib/Dialect/SV/SVOps.cpp +++ b/lib/Dialect/SV/SVOps.cpp @@ -390,8 +390,7 @@ void IfDefOp::build(OpBuilder &builder, OperationState &result, } LogicalResult IfDefOp::verifySymbolUses(SymbolTableCollection &symbolTable) { - // TODO(nandor): Pending fixes to the pipeline, verify references. - return success(); + return verifyMacroIdentSymbolUses(*this, getCond().getIdent(), symbolTable); } // If both thenRegion and elseRegion are empty, erase op. @@ -464,8 +463,7 @@ LogicalResult IfDefProceduralOp::canonicalize(IfDefProceduralOp op, LogicalResult IfDefProceduralOp::verifySymbolUses(SymbolTableCollection &symbolTable) { - // TODO(nandor): Pending fixes to the pipeline, verify references. - return success(); + return verifyMacroIdentSymbolUses(*this, getCond().getIdent(), symbolTable); } //===----------------------------------------------------------------------===// diff --git a/test/Dialect/SV/hw-extract-test-code.mlir b/test/Dialect/SV/hw-extract-test-code.mlir index 3cf15873282..90c07ba282a 100644 --- a/test/Dialect/SV/hw-extract-test-code.mlir +++ b/test/Dialect/SV/hw-extract-test-code.mlir @@ -1,5 +1,6 @@ // RUN: circt-opt --sv-extract-test-code --split-input-file %s | FileCheck %s // CHECK-LABEL: module attributes {firrtl.extract.assert = #hw.output_file<"dir3{{/|\\\\}}" +// CHECK-NEXT: sv.macro.decl @SYNTHESIS // CHECK-NEXT: hw.module.extern @foo_cover // CHECK-NOT: attributes // CHECK-NEXT: hw.module.extern @foo_assume @@ -33,6 +34,7 @@ // CHECK: sv.bind <@issue1246::@__ETC_issue1246_assume> {output_file = #hw.output_file<"file4", excludeFromFileList>} // CHECK: sv.bind <@issue1246::@__ETC_issue1246_cover> module attributes {firrtl.extract.assert = #hw.output_file<"dir3/", excludeFromFileList, includeReplicatedOps>, firrtl.extract.assume.bindfile = #hw.output_file<"file4", excludeFromFileList>} { + sv.macro.decl @SYNTHESIS 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"} @@ -292,6 +294,8 @@ module { // CHECK: hw.instance "non_testcode_and_instance1" module { + sv.macro.decl @SYNTHESIS + hw.module private @Foo(in %a: i1, out b: i1) { hw.output %a : i1 } diff --git a/test/Dialect/Seq/firreg.mlir b/test/Dialect/Seq/firreg.mlir index b7e826c2ef4..7e066f46d9b 100644 --- a/test/Dialect/Seq/firreg.mlir +++ b/test/Dialect/Seq/firreg.mlir @@ -1,6 +1,6 @@ // RUN: circt-opt %s -verify-diagnostics --lower-seq-to-sv | FileCheck %s --check-prefixes=CHECK,COMMON -// RUN: circt-opt %s -verify-diagnostics --pass-pipeline="builtin.module(lower-seq-to-sv{disable-reg-randomization})" | FileCheck %s --check-prefix COMMON --implicit-check-not RANDOMIZE_REG -// RUN: circt-opt %s -verify-diagnostics --pass-pipeline="builtin.module(lower-seq-to-sv{emit-separate-always-blocks})" | FileCheck %s --check-prefixes SEPARATE +// RUN: circt-opt %s -verify-diagnostics --pass-pipeline="builtin.module(lower-seq-to-sv{disable-reg-randomization})" | FileCheck %s --check-prefixes=COMMON,DISABLED +// RUN: circt-opt %s -verify-diagnostics --pass-pipeline="builtin.module(lower-seq-to-sv{emit-separate-always-blocks})" | FileCheck %s --check-prefixes=SEPARATE // COMMON-LABEL: hw.module @lowering // SEPARATE-LABEL: hw.module @lowering @@ -103,6 +103,7 @@ hw.module @lowering(in %clk : !seq.clock, in %rst : i1, in %in : i32, out a : i3 // SEPARATE-NEXT: sv.passign %rNoSym, %in : i32 // SEPARATE-NEXT: } + // DISABLED-NOT: sv.ifdef.procedural @RANDOMIZE_REG // CHECK: sv.ifdef @ENABLE_INITIAL_REG_ { // CHECK-NEXT: sv.ordered { // CHECK-NEXT: sv.ifdef @FIRRTL_BEFORE_INITIAL { @@ -183,6 +184,7 @@ hw.module private @UninitReg1(in %clock : !seq.clock, in %reset : i1, in %cond : %1 = comb.mux bin %cond, %value, %count : i2 %2 = comb.mux bin %reset, %c0_i2, %1 : i2 + // DISABLED-NOT: sv.ifdef.procedural @RANDOMIZE_REG // CHECK-NEXT: sv.ifdef @ENABLE_INITIAL_REG_ { // CHECK-NEXT: sv.ordered { // CHECK-NEXT: sv.ifdef @FIRRTL_BEFORE_INITIAL { @@ -262,6 +264,7 @@ hw.module private @InitReg1(in %clock: !seq.clock, in %reset: i1, in %io_d: i32, %3 = comb.extract %2 from 1 : (i33) -> i32 %4 = comb.mux bin %io_en, %io_d, %3 : i32 + // DISABLED-NOT: sv.ifdef.procedural @RANDOMIZE_REG // COMMON: %reg = sv.reg sym @[[reg_sym:.+]] : !hw.inout // COMMON-NEXT: %0 = sv.read_inout %reg : !hw.inout // COMMON-NEXT: %reg2 = sv.reg sym @[[reg2_sym:.+]] : !hw.inout @@ -339,6 +342,7 @@ hw.module private @UninitReg42(in %clock: !seq.clock, in %reset: i1, in %cond: i %0 = comb.mux %cond, %value, %count : i42 %1 = comb.mux %reset, %c0_i42, %0 : i42 + // DISABLED-NOT: sv.ifdef.procedural @RANDOMIZE_REG // CHECK: %count = sv.reg sym @count : !hw.inout // CHECK: sv.ifdef @ENABLE_INITIAL_REG_ { // CHECK-NEXT: sv.ordered { @@ -385,6 +389,7 @@ hw.module private @init1DVector(in %clock: !seq.clock, in %a: !hw.array<2xi1>, o // CHECK-NEXT: sv.passign %r, %a : !hw.array<2xi1> // CHECK-NEXT: } + // DISABLED-NOT: sv.ifdef.procedural @RANDOMIZE_REG // CHECK: sv.ifdef @ENABLE_INITIAL_REG_ { // CHECK-NEXT: sv.ordered { // CHECK-NEXT: sv.ifdef @FIRRTL_BEFORE_INITIAL { @@ -428,6 +433,7 @@ hw.module private @init1DVector(in %clock: !seq.clock, in %a: !hw.array<2xi1>, o hw.module private @init2DVector(in %clock: !seq.clock, in %a: !hw.array<1xarray<1xi1>>, out b: !hw.array<1xarray<1xi1>>) { %r = seq.firreg %a clock %clock sym @__r__ : !hw.array<1xarray<1xi1>> + // DISABLED-NOT: sv.ifdef.procedural @RANDOMIZE_REG // CHECK: sv.always posedge %clock { // CHECK-NEXT: sv.passign %r, %a : !hw.array<1xarray<1xi1>> // CHECK-NEXT: } @@ -471,6 +477,7 @@ hw.module private @initStruct(in %clock: !seq.clock) { %r = seq.firreg %r clock %clock sym @__r__ : !hw.struct // CHECK: %r = sv.reg sym @[[r_sym:[_A-Za-z0-9]+]] + // DISABLED-NOT: sv.ifdef.procedural @RANDOMIZE_REG // CHECK: sv.ifdef @ENABLE_INITIAL_REG_ { // CHECK-NEXT: sv.ordered { // CHECK-NEXT: sv.ifdef @FIRRTL_BEFORE_INITIAL { diff --git a/test/Dialect/Seq/hw-memsim.mlir b/test/Dialect/Seq/hw-memsim.mlir index c2328e8602e..f15c6a4f3ed 100644 --- a/test/Dialect/Seq/hw-memsim.mlir +++ b/test/Dialect/Seq/hw-memsim.mlir @@ -2,14 +2,17 @@ // RUN: circt-opt -pass-pipeline="builtin.module(hw-memory-sim{read-enable-mode=ignore})" %s | FileCheck %s --check-prefixes=COMMON,IGNORE // RUN: circt-opt -pass-pipeline="builtin.module(hw-memory-sim{read-enable-mode=zero})" %s | FileCheck %s --check-prefixes=COMMON,ZERO // RUN: circt-opt -pass-pipeline="builtin.module(hw-memory-sim{add-mux-pragmas})" %s | FileCheck %s --check-prefixes=COMMON,PRAMGAS -// RUN: circt-opt -pass-pipeline="builtin.module(hw-memory-sim{disable-mem-randomization})" %s | FileCheck %s --check-prefix COMMON --implicit-check-not RANDOMIZE_MEM -// RUN: circt-opt -pass-pipeline="builtin.module(hw-memory-sim{disable-reg-randomization})" %s | FileCheck %s --check-prefix COMMON --implicit-check-not RANDOMIZE_REG -// RUN: circt-opt -pass-pipeline="builtin.module(hw-memory-sim{disable-mem-randomization disable-reg-randomization})" %s | FileCheck %s --check-prefix COMMON --implicit-check-not RANDOMIZE_REG --implicit-check-not RANDOMIZE_MEM +// RUN: circt-opt -pass-pipeline="builtin.module(hw-memory-sim{disable-mem-randomization})" %s | FileCheck %s --check-prefix COMMON --check-prefix NO-RAND-MEM +// RUN: circt-opt -pass-pipeline="builtin.module(hw-memory-sim{disable-reg-randomization})" %s | FileCheck %s --check-prefix COMMON --check-prefix NO-RAND-REG +// RUN: circt-opt -pass-pipeline="builtin.module(hw-memory-sim{disable-mem-randomization disable-reg-randomization})" %s | FileCheck %s --check-prefixes NO-RAND-MEM,NO-RAND-REG // RUN: circt-opt -pass-pipeline="builtin.module(hw-memory-sim{add-vivado-ram-address-conflict-synthesis-bug-workaround})" %s | FileCheck %s --check-prefixes=CHECK,COMMON,VIVADO hw.generator.schema @FIRRTLMem, "FIRRTL_Memory", ["depth", "numReadPorts", "numWritePorts", "numReadWritePorts", "readLatency", "writeLatency", "width", "readUnderWrite", "writeUnderWrite", "writeClockIDs", "initFilename", "initIsBinary", "initIsInline"] sv.macro.decl @RANDOM +sv.macro.decl @ENABLE_INITIAL_MEM_ +sv.macro.decl @RANDOMIZE_REG_INIT +sv.macro.decl @RANDOMIZE_MEM_INIT // COMMON-LABEL: @complex hw.module @complex(in %clock: i1, in %reset: i1, in %r0en: i1, in %mode: i1, in %data0: i16, out data1: i16, out data2: i16) { @@ -77,6 +80,9 @@ hw.module @WriteOrderedDifferentClock(in %clock: i1, in %clock2: i1, in %w0_addr hw.module.generated @FIRRTLMem_1_1_1_16_10_0_1_0_0, @FIRRTLMem(in %ro_addr_0: i4, in %ro_en_0: i1, in %ro_clock_0: i1, in %rw_addr_0: i4, in %rw_en_0: i1, in %rw_clock_0: i1, in %rw_wmode_0: i1, in %rw_wdata_0: i16, in %wo_addr_0: i4, in %wo_en_0: i1, in %wo_clock_0: i1, in %wo_data_0: i16, out ro_data_0: i16, out rw_rdata_0: i16) attributes {depth = 10 : i64, numReadPorts = 1 : ui32, numReadWritePorts = 1 : ui32, numWritePorts = 1 : ui32, readLatency = 0 : ui32, readUnderWrite = 0 : i32, width = 16 : ui32, writeClockIDs = [], writeLatency = 1 : ui32, writeUnderWrite = 0 : i32, initFilename = "", initIsBinary = false, initIsInline = false} +// NO-RAND-MEM-NOT: sv.ifdef.procedural @RANDOMIZE_MEM +// NO-RAND-REG-NOT: sv.ifdef.procedural @RANDOMIZE_REG + //COMMON-LABEL: @FIRRTLMem_1_1_1_16_10_0_1_0_0 //CHECK-SAME: attributes {comment = "VCS coverage exclude_file"} //CHECK: %Memory = sv.reg diff --git a/test/firtool/firtool.fir b/test/firtool/firtool.fir index 5866144048e..86b2f7f47e9 100644 --- a/test/firtool/firtool.fir +++ b/test/firtool/firtool.fir @@ -44,14 +44,14 @@ circuit test_mod : %[[{"class": "circt.testNT", "data": "a"}]] ; OMIROUT-NEXT: "value": "OMString:hello" ; OMIROUT-NEXT: } -; VERILOG-IR: sv.macro.decl @ENABLE_INITIAL_MEM_ +; VERILOG-IR: sv.verbatim{{.*}}Standard header to adapt well known macros for register randomization ; VERILOG-IR: sv.macro.decl @ENABLE_INITIAL_REG_ -; VERILOG-IR: sv.macro.decl @INIT_RANDOM_PROLOG_ +; VERILOG-IR: sv.macro.decl @ENABLE_INITIAL_MEM_ +; VERILOG-IR: sv.macro.decl @RANDOMIZE ; VERILOG-IR: sv.macro.decl @RANDOMIZE_DELAY -; VERILOG-IR: sv.macro.decl @INIT_RANDOM ; VERILOG-IR: sv.macro.decl @RANDOM -; VERILOG-IR: sv.macro.decl @RANDOMIZE -; VERILOG-IR: sv.verbatim{{.*}}Standard header to adapt well known macros for register randomization +; VERILOG-IR: sv.macro.decl @INIT_RANDOM +; VERILOG-IR: sv.macro.decl @INIT_RANDOM_PROLOG_ type V3 = UInt<1>[3] type V2 = UInt<1>[2]