From 9775df2cb438c50907c3c46e659aca500eacbce5 Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Tue, 17 Dec 2024 13:11:34 -0700 Subject: [PATCH] [FIRRTL] Support MarkDUTAnnotation on extmodules. (#8001) In some cases, the "DUT" might be an extmodule from a separate compilation unit, and we still want all the old legacy "is DUT" logic to work. To support this, we need applyDUTAnno to allow the annotation to be applied to an extmodule, and we need extractDUT and its users to work with FModuleLikes instead of FModuleOp. The only other thing that appears to be using this "is DUT" logic is the newer InstanceInfo helper, which already works with igraph::ModuleOpInterfaces and seems to work fine with extmodules as the "DUT". --- .../circt/Dialect/FIRRTL/FIRRTLAnnotations.h | 2 +- lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp | 3 +- .../FIRRTL/Transforms/BlackBoxReader.cpp | 4 +- .../FIRRTL/Transforms/GrandCentral.cpp | 4 +- .../FIRRTL/Transforms/LowerAnnotations.cpp | 4 +- test/Analysis/firrtl-test-instance-info.mlir | 37 +++++++++++++++++++ test/firtool/mark-dut.fir | 29 +++++++++++++++ 7 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 test/firtool/mark-dut.fir diff --git a/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h b/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h index 4682a05ff6..29709bdff8 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h +++ b/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h @@ -448,7 +448,7 @@ struct PortAnnoTarget : public AnnoTarget { /// found or if the DUT was found and a previous DUT was not set (if `dut` is /// null). This returns failure if a DUT was found and a previous DUT was set. /// This function generates an error message in the failure case. -LogicalResult extractDUT(FModuleOp mod, FModuleOp &dut); +LogicalResult extractDUT(FModuleLike mod, FModuleLike &dut); } // namespace firrtl } // namespace circt diff --git a/lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp b/lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp index d5dc2b29f0..a733d32d3a 100644 --- a/lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp @@ -564,7 +564,8 @@ FIRRTLType PortAnnoTarget::getType() const { // TODO: Remove these in favor of first-class annotations. //===----------------------------------------------------------------------===// -LogicalResult circt::firrtl::extractDUT(const FModuleOp mod, FModuleOp &dut) { +LogicalResult circt::firrtl::extractDUT(const FModuleLike mod, + FModuleLike &dut) { if (!AnnotationSet(mod).hasAnnotation(dutAnnoClass)) return success(); diff --git a/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp b/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp index 0cfa24d1b9..42907d9511 100644 --- a/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp +++ b/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp @@ -104,7 +104,7 @@ private: /// The design-under-test (DUT) as indicated by the presence of a /// "sifive.enterprise.firrtl.MarkDUTAnnotation". This will be null if no /// annotation is present. - FModuleOp dut; + FModuleLike dut; /// The file list file name (sic) for black boxes. If set, generates a file /// that lists all non-header source files for black boxes. Can be changed @@ -207,7 +207,7 @@ void BlackBoxReaderPass::runOnOperation() { // Do a shallow walk of the circuit to collect information necessary before we // do real work. for (auto &op : *circuitOp.getBodyBlock()) { - FModuleOp module = dyn_cast(op); + FModuleLike module = dyn_cast(op); // Find the DUT if it exists or error if there are multiple DUTs. if (module) if (failed(extractDUT(module, dut))) diff --git a/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp b/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp index 92d448691a..fdd4853ac5 100644 --- a/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp +++ b/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp @@ -613,7 +613,7 @@ private: /// The design-under-test (DUT) as determined by the presence of a /// "sifive.enterprise.firrtl.MarkDUTAnnotation". This will be null if no DUT /// was found. - FModuleOp dut; + FModuleLike dut; /// An optional directory for testbench-related files. This is null if no /// "TestBenchDirAnnotation" is found. @@ -1577,7 +1577,7 @@ void GrandCentralPass::runOnOperation() { // Find the DUT if it exists. This needs to be known before the circuit is // walked. - for (auto mod : circuitOp.getOps()) { + for (auto mod : circuitOp.getOps()) { if (failed(extractDUT(mod, dut))) removalError = true; } diff --git a/lib/Dialect/FIRRTL/Transforms/LowerAnnotations.cpp b/lib/Dialect/FIRRTL/Transforms/LowerAnnotations.cpp index 178a257f6b..d8ae6b45b9 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerAnnotations.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerAnnotations.cpp @@ -246,10 +246,10 @@ static LogicalResult applyDUTAnno(const AnnoPathValue &target, if (!target.isLocal()) return mlir::emitError(loc) << "must be local"; - if (!isa(target.ref) || !isa(op)) + if (!isa(target.ref) || !isa(op)) return mlir::emitError(loc) << "can only target to a module"; - auto moduleOp = cast(op); + auto moduleOp = cast(op); // DUT has public visibility. moduleOp.setPublic(); diff --git a/test/Analysis/firrtl-test-instance-info.mlir b/test/Analysis/firrtl-test-instance-info.mlir index ad38029077..ec38da6a24 100644 --- a/test/Analysis/firrtl-test-instance-info.mlir +++ b/test/Analysis/firrtl-test-instance-info.mlir @@ -487,3 +487,40 @@ firrtl.circuit "Foo" { firrtl.instance a {lowerToBind} @Foo_A() } } + +// ----- + +// Test that the DUT can be an extmodule +// CHECK: - operation: firrtl.circuit "Testharness" +// CHECK-NEXT: hasDut: true +// CHECK-NEXT: dut: firrtl.extmodule private @DUT +// CHECK-NEXT: effectiveDut: firrtl.extmodule private @DUT +firrtl.circuit "Testharness" { + // CHECK: - operation: firrtl.module @Testharness + // CHECK-NEXT: isDut: false + // CHECK-NEXT: anyInstanceUnderDut: false + // CHECK-NEXT: allInstancesUnderDut: false + firrtl.module @Testharness() { + firrtl.instance dut @DUT() + firrtl.instance foo @Foo() + } + + // CHECK: - operation: firrtl.extmodule private @DUT + // CHECK-NEXT: isDut: true + // CHECK-NEXT: anyInstanceUnderDut: true + // CHECK-NEXT: allInstancesUnderDut: true + firrtl.extmodule private @DUT() attributes { + annotations = [ + { + class = "sifive.enterprise.firrtl.MarkDUTAnnotation" + } + ] + } + + // CHECK: - operation: firrtl.module private @Foo + // CHECK-NEXT: isDut: false + // CHECK-NEXT: anyInstanceUnderDut: false + // CHECK-NEXT: allInstancesUnderDut: false + firrtl.module private @Foo() { + } +} diff --git a/test/firtool/mark-dut.fir b/test/firtool/mark-dut.fir new file mode 100644 index 0000000000..3161a3ad47 --- /dev/null +++ b/test/firtool/mark-dut.fir @@ -0,0 +1,29 @@ +; RUN: firtool %s -ir-verilog | FileCheck %s + +FIRRTL version 4.1.0 + +; COM: Check that we can even target an extmodule. +circuit Test : %[[ + { + "class": "sifive.enterprise.firrtl.MarkDUTAnnotation", + "target": "~Test|DUT" + } +]] + ; CHECK: hw.hierpath private [[DUT_NLA:@.+]] [@Test::[[DUT_SYM:@.+]]] + public module Test : + input in : UInt<1> + output out : UInt<1> + + ; CHECK: hw.instance "dut" sym [[DUT_SYM]] + inst dut of DUT + + connect dut.in, in + connect out, dut.out + + extmodule DUT : + input in : UInt<1> + output out : UInt<1> + + ; COM: Check that metadata includes the dutModulePath pointing to Test::dut. + ; CHECK: om.class @SiFive_Metadata(%basepath: !om.basepath) -> (dutModulePath + ; CHECK-NEXT: om.path_create instance %basepath [[DUT_NLA]]