Skip to content

Commit

Permalink
[FIRRTL] Verify RWProbeOp target has layer requirements. (#7372)
Browse files Browse the repository at this point in the history
RWProbe conservatively means a write to the target, so check
that the target is indeed writeable from where the rwprobe is.
  • Loading branch information
dtzSiFive authored Aug 6, 2024
1 parent 0b93783 commit 4415b9c
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 3 deletions.
26 changes: 23 additions & 3 deletions lib/Dialect/FIRRTL/FIRRTLOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6201,10 +6201,30 @@ LogicalResult RWProbeOp::verifyInnerRefs(hw::InnerRefNamespace &ns) {
}
return success();
};

auto checkLayers = [&](Location loc) -> LogicalResult {
auto dstLayers = getAmbientLayersAt(target.getOp());
auto srcLayers = getLayersFor(getResult());
SmallVector<SymbolRefAttr> missingLayers;
if (!isLayerSetCompatibleWith(srcLayers, dstLayers, missingLayers)) {
auto diag = emitOpError("target has insufficient layer requirements");
auto &note = diag.attachNote(loc);
note << "target is missing layer requirements: ";
llvm::interleaveComma(missingLayers, note);
return failure();
}
return success();
};
auto checks = [&](auto type, Location loc) {
if (failed(checkLayers(loc)))
return failure();
return checkFinalType(type, loc);
};

if (target.isPort()) {
auto mod = cast<FModuleLike>(target.getOp());
return checkFinalType(mod.getPortType(target.getPort()),
mod.getPortLocation(target.getPort()));
return checks(mod.getPortType(target.getPort()),
mod.getPortLocation(target.getPort()));
}
hw::InnerSymbolOpInterface symOp =
cast<hw::InnerSymbolOpInterface>(target.getOp());
Expand All @@ -6218,7 +6238,7 @@ LogicalResult RWProbeOp::verifyInnerRefs(hw::InnerRefNamespace &ns) {
return emitOpError("is not dominated by target")
.attachNote(symOp.getLoc())
.append("target here");
return checkFinalType(symOp.getTargetResult().getType(), symOp.getLoc());
return checks(symOp.getTargetResult().getType(), symOp.getLoc());
}

//===----------------------------------------------------------------------===//
Expand Down
14 changes: 14 additions & 0 deletions test/Dialect/FIRRTL/errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -2072,6 +2072,20 @@ firrtl.circuit "RWProbeUseDef" {

// -----

firrtl.circuit "RWProbeLayerRequirements" {
firrtl.layer @A bind { }
firrtl.module @RWProbeLayerRequirements(in %cond : !firrtl.uint<1>) {
// expected-note @below {{target is missing layer requirements: @A}}
%w = firrtl.wire sym @x : !firrtl.uint<1>
firrtl.layerblock @A {
// expected-error @below {{target has insufficient layer requirements}}
%rw = firrtl.ref.rwprobe <@RWProbeLayerRequirements::@x> : !firrtl.rwprobe<uint<1>>
}
}
}

// -----

firrtl.circuit "MissingClassForObjectPortInModule" {
// expected-error @below {{'firrtl.module' op references unknown class @Missing}}
firrtl.module @MissingClassForObjectPortInModule(out %o: !firrtl.class<@Missing()>) {}
Expand Down
12 changes: 12 additions & 0 deletions test/Dialect/FIRRTL/layers.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,16 @@ firrtl.circuit "Test" {
firrtl.propassign %foo_in, %str : !firrtl.string
}
}

//===--------------------------------------------------------------------===//
// RWProbe under Layer
//===--------------------------------------------------------------------===//

firrtl.module @RWProbeInLayer() {
firrtl.layerblock @A {
%w = firrtl.wire sym @sym : !firrtl.uint<1>
%rwp = firrtl.ref.rwprobe <@RWProbeInLayer::@sym> : !firrtl.rwprobe<uint<1>>
}
}

}

0 comments on commit 4415b9c

Please sign in to comment.