Skip to content

Commit

Permalink
[FIRRTL] Add verifier of single MarkDUTAnnotation (#7633)
Browse files Browse the repository at this point in the history
Add a CircuitOp verifier that guarantees that there is zero or one modules
annotated with a `MarkDUTAnnotation`.  This is important as many passes
assume that this is the case.  Moving this check into a verifier means
that logic checking this behavior in each pass can be removed.  I am
slightly concerned of the performance cost of doing this as examining the
annotations is non-trivial and this will cause that examination to now
happen more often.

Signed-off-by: Schuyler Eldridge <[email protected]>
  • Loading branch information
seldridge authored Sep 26, 2024
1 parent ee8e605 commit 3ffc39c
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
19 changes: 19 additions & 0 deletions lib/Dialect/FIRRTL/FIRRTLOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//

#include "circt/Dialect/FIRRTL/FIRRTLOps.h"
#include "circt/Dialect/FIRRTL/AnnotationDetails.h"
#include "circt/Dialect/FIRRTL/CHIRRTLDialect.h"
#include "circt/Dialect/FIRRTL/FIRRTLAnnotations.h"
#include "circt/Dialect/FIRRTL/FIRRTLAttributes.h"
Expand Down Expand Up @@ -627,14 +628,32 @@ LogicalResult CircuitOp::verifyRegions() {
return success();
};

SmallVector<FModuleOp, 1> dutModules;
for (auto &op : *getBodyBlock()) {
// Verify modules.
if (auto moduleOp = dyn_cast<FModuleOp>(op)) {
if (AnnotationSet(moduleOp).hasAnnotation(dutAnnoClass))
dutModules.push_back(moduleOp);
continue;
}

// Verify external modules.
if (auto extModule = dyn_cast<FExtModuleOp>(op)) {
if (verifyExtModule(extModule).failed())
return failure();
}
}

// Error if there is more than one design-under-test.
if (dutModules.size() > 1) {
auto diag = dutModules[0]->emitOpError()
<< "is annotated as the design-under-test (DUT), but other "
"modules are also annotated";
for (auto moduleOp : ArrayRef(dutModules).drop_front())
diag.attachNote(moduleOp.getLoc()) << "is also annotated as the DUT";
return failure();
}

return success();
}

Expand Down
30 changes: 30 additions & 0 deletions test/Dialect/FIRRTL/errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -2561,3 +2561,33 @@ firrtl.circuit "MainNotModule" {
return
}
}

// -----

firrtl.circuit "MultipleDUTModules" {
firrtl.module @MultipleDUTModules() {}
// expected-error @below {{is annotated as the design-under-test}}
firrtl.module @Foo() attributes {
annotations = [
{
class = "sifive.enterprise.firrtl.MarkDUTAnnotation"
}
]
} {}
// expected-note @below {{is also annotated as the DUT}}
firrtl.module @Bar() attributes {
annotations = [
{
class = "sifive.enterprise.firrtl.MarkDUTAnnotation"
}
]
} {}
// expected-note @below {{is also annotated as the DUT}}
firrtl.module @Baz() attributes {
annotations = [
{
class = "sifive.enterprise.firrtl.MarkDUTAnnotation"
}
]
} {}
}

0 comments on commit 3ffc39c

Please sign in to comment.