Skip to content

Commit

Permalink
[FIRRTL] InferResets: verify that FART annotation is on async resets (#…
Browse files Browse the repository at this point in the history
…6674)

The FullAsynchronousResetAnnotation is used to mark an asynchronous
reset signal in a module to be used as the reset for all registers that
do not already have an async reset in this module and below in the
hierarchy. There was no error checking that this annotation was attached
to an asynchronous reset typed signal, and the pass would accidentally
try to wire whatever signal was annotated. In some cases, this could
lead to a connecting the signal to an async typed port, which would
become a type checking error in the verifier. This changes InferResets
to verify that the FART annotated signal is in fact asynchronous.
  • Loading branch information
youngar authored Feb 8, 2024
1 parent 26ac773 commit 641ae64
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 0 deletions.
16 changes: 16 additions & 0 deletions lib/Dialect/FIRRTL/Transforms/InferResets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1299,8 +1299,16 @@ InferResetsPass::collectAnnos(FModuleOp module) {
Annotation anno) {
Value arg = module.getArgument(argNum);
if (anno.isClass(fullAsyncResetAnnoClass)) {
if (!isa<AsyncResetType>(arg.getType())) {
mlir::emitError(arg.getLoc(), "'IgnoreFullAsyncResetAnnotation' must "
"target async reset, but targets ")
<< arg.getType();
anyFailed = true;
return true;
}
reset = arg;
conflictingAnnos.insert({anno, reset.getLoc()});

return true;
}
if (anno.isClass(ignoreFullAsyncResetAnnoClass)) {
Expand Down Expand Up @@ -1332,7 +1340,15 @@ InferResetsPass::collectAnnos(FModuleOp module) {

// At this point we know that we have a WireOp/NodeOp. Process the reset
// annotations.
auto resultType = op->getResult(0).getType();
if (anno.isClass(fullAsyncResetAnnoClass)) {
if (!isa<AsyncResetType>(resultType)) {
mlir::emitError(op->getLoc(), "'IgnoreFullAsyncResetAnnotation' must "
"target async reset, but targets ")
<< resultType;
anyFailed = true;
return true;
}
reset = op->getResult(0);
conflictingAnnos.insert({anno, reset.getLoc()});
return true;
Expand Down
21 changes: 21 additions & 0 deletions test/Dialect/FIRRTL/infer-resets-errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,27 @@ firrtl.circuit "top" {
}
}

// -----
// Reset annotation cannot target synchronous reset signals
firrtl.circuit "top" {
firrtl.module @top() {
// expected-error @below {{'IgnoreFullAsyncResetAnnotation' must target async reset, but targets '!firrtl.uint<1>'}}
%innerReset = firrtl.wire {annotations = [{class = "sifive.enterprise.firrtl.FullAsyncResetAnnotation"}]} : !firrtl.uint<1>
}
}

// -----
// Reset annotation cannot target reset signals which are inferred to be synchronous
firrtl.circuit "top" {
firrtl.module @top() {
// expected-error @below {{'IgnoreFullAsyncResetAnnotation' must target async reset, but targets '!firrtl.uint<1>'}}
%innerReset = firrtl.wire {annotations = [{class = "sifive.enterprise.firrtl.FullAsyncResetAnnotation"}]} : !firrtl.reset
%invalid = firrtl.invalidvalue : !firrtl.reset
firrtl.strictconnect %innerReset, %invalid : !firrtl.reset
}
}


// -----
// Ignore reset annotation cannot target port
firrtl.circuit "top" {
Expand Down

0 comments on commit 641ae64

Please sign in to comment.