Skip to content

Commit

Permalink
[FIRRTL] Support MarkDUTAnnotation on extmodules. (#8001)
Browse files Browse the repository at this point in the history
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".
  • Loading branch information
mikeurbach authored Dec 17, 2024
1 parent 50e3f9f commit 9775df2
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 8 deletions.
2 changes: 1 addition & 1 deletion include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
4 changes: 2 additions & 2 deletions lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ struct BlackBoxReaderPass
/// 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
Expand Down Expand Up @@ -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<FModuleOp>(op);
FModuleLike module = dyn_cast<FModuleLike>(op);
// Find the DUT if it exists or error if there are multiple DUTs.
if (module)
if (failed(extractDUT(module, dut)))
Expand Down
4 changes: 2 additions & 2 deletions lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ struct GrandCentralPass
/// 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.
Expand Down Expand Up @@ -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<FModuleOp>()) {
for (auto mod : circuitOp.getOps<FModuleLike>()) {
if (failed(extractDUT(mod, dut)))
removalError = true;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/Dialect/FIRRTL/Transforms/LowerAnnotations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,10 @@ static LogicalResult applyDUTAnno(const AnnoPathValue &target,
if (!target.isLocal())
return mlir::emitError(loc) << "must be local";

if (!isa<OpAnnoTarget>(target.ref) || !isa<FModuleOp>(op))
if (!isa<OpAnnoTarget>(target.ref) || !isa<FModuleLike>(op))
return mlir::emitError(loc) << "can only target to a module";

auto moduleOp = cast<FModuleOp>(op);
auto moduleOp = cast<FModuleLike>(op);

// DUT has public visibility.
moduleOp.setPublic();
Expand Down
37 changes: 37 additions & 0 deletions test/Analysis/firrtl-test-instance-info.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
}
}
29 changes: 29 additions & 0 deletions test/firtool/mark-dut.fir
Original file line number Diff line number Diff line change
@@ -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]]

0 comments on commit 9775df2

Please sign in to comment.