Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIRRTL] Optionally look through casts for fieldref chasing. #6093

Merged
merged 1 commit into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions include/circt/Dialect/FIRRTL/FIRRTLUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,9 @@ bool walkDrivers(FIRRTLBaseValue value, bool lookThroughWires,

/// Get the FieldRef from a value. This will travel backwards to through the
/// IR, following Subfield and Subindex to find the op which declares the
/// location.
FieldRef getFieldRefFromValue(Value value);
/// location. Optionally look through recognized cast operations, which
/// likely will result in source having slightly different type.
FieldRef getFieldRefFromValue(Value value, bool lookThroughCasts = false);

/// Get a string identifier representing the FieldRef. Return this string and a
/// boolean indicating if a valid "root" for the identifier was found. If
Expand Down
4 changes: 2 additions & 2 deletions lib/Dialect/FIRRTL/FIRRTLOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2941,7 +2941,7 @@ static LogicalResult checkConnectFlow(Operation *connect) {
// A sink that is a port output or instance input used as a source is okay.
auto kind = getDeclarationKind(src);
if (kind != DeclKind::Port && kind != DeclKind::Instance) {
auto srcRef = getFieldRefFromValue(src);
auto srcRef = getFieldRefFromValue(src, /*lookThroughCasts=*/true);
auto [srcName, rootKnown] = getFieldName(srcRef);
auto diag = emitError(connect->getLoc());
diag << "connect has invalid flow: the source expression ";
Expand All @@ -2954,7 +2954,7 @@ static LogicalResult checkConnectFlow(Operation *connect) {

auto dstFlow = foldFlow(dst);
if (!isValidDst(dstFlow)) {
auto dstRef = getFieldRefFromValue(dst);
auto dstRef = getFieldRefFromValue(dst, /*lookThroughCasts=*/true);
auto [dstName, rootKnown] = getFieldName(dstRef);
auto diag = emitError(connect->getLoc());
diag << "connect has invalid flow: the destination expression ";
Expand Down
9 changes: 8 additions & 1 deletion lib/Dialect/FIRRTL/FIRRTLUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,8 @@ bool circt::firrtl::walkDrivers(FIRRTLBaseValue value, bool lookThroughWires,
// FieldRef helpers
//===----------------------------------------------------------------------===//

FieldRef circt::firrtl::getFieldRefFromValue(Value value) {
FieldRef circt::firrtl::getFieldRefFromValue(Value value,
bool lookThroughCasts) {
// This code walks upwards from the subfield and calculates the field ID at
// each level. At each stage, it must take the current id, and re-index it as
// a nested bundle under the parent field.. This is accomplished by using the
Expand All @@ -488,6 +489,12 @@ FieldRef circt::firrtl::getFieldRefFromValue(Value value) {

auto handled =
TypeSwitch<Operation *, bool>(op)
.Case<RefCastOp, ConstCastOp, UninferredResetCastOp>([&](auto op) {
if (!lookThroughCasts)
return false;
value = op.getInput();
return true;
})
.Case<SubfieldOp, OpenSubfieldOp>([&](auto subfieldOp) {
value = subfieldOp.getInput();
typename decltype(subfieldOp)::InputType bundleType =
Expand Down
13 changes: 10 additions & 3 deletions lib/Dialect/FIRRTL/Transforms/Dedup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -683,12 +683,19 @@ struct Equivalence {
if (bValue != data.map.lookup(aValue)) {
diag.attachNote(a->getLoc())
<< "operations use different operands, first operand is '"
<< getFieldName(getFieldRefFromValue(aValue)).first << "'";
<< getFieldName(
getFieldRefFromValue(aValue, /*lookThroughCasts=*/true))
.first
<< "'";
diag.attachNote(b->getLoc())
<< "second operand is '"
<< getFieldName(getFieldRefFromValue(bValue)).first
<< getFieldName(
getFieldRefFromValue(bValue, /*lookThroughCasts=*/true))
.first
<< "', but should have been '"
<< getFieldName(getFieldRefFromValue(data.map.lookup(aValue))).first
<< getFieldName(getFieldRefFromValue(data.map.lookup(aValue),
/*lookThroughCasts=*/true))
.first
<< "'";
return failure();
}
Expand Down
4 changes: 3 additions & 1 deletion lib/Dialect/FIRRTL/Transforms/LowerAnnotations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,9 @@ LogicalResult LowerAnnotationsPass::solveWiringProblems(ApplyState &state) {
if (name.empty()) {
if (problem.newNameHint.empty())
name = state.getNamespace(mod).newName(
getFieldName(getFieldRefFromValue(val), /*nameSafe=*/true)
getFieldName(
getFieldRefFromValue(val, /*lookThroughCasts=*/true),
/*nameSafe=*/true)
.first +
"__bore");
else
Expand Down
3 changes: 2 additions & 1 deletion lib/Dialect/FIRRTL/Transforms/LowerXMR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ class LowerXMRPass : public LowerXMRBase<LowerXMRPass> {
auto nameKind = NameKindEnum::DroppableName;

if (auto [name, rootKnown] = getFieldName(
getFieldRefFromValue(xmrDef), /*nameSafe=*/true);
getFieldRefFromValue(xmrDef, /*lookThroughCasts=*/true),
/*nameSafe=*/true);
rootKnown) {
opName = name + "_probe";
nameKind = NameKindEnum::InterestingName;
Expand Down
6 changes: 4 additions & 2 deletions test/Dialect/FIRRTL/errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -1237,12 +1237,14 @@ firrtl.circuit "NoDefineIntoRefSub" {
// Can't define into a ref.cast.

firrtl.circuit "NoDefineIntoRefCast" {
firrtl.module @NoDefineIntoRefCast(out %r: !firrtl.probe<uint<1>>) {
firrtl.module @NoDefineIntoRefCast(
// expected-note @below {{the destination was defined here}}
out %r: !firrtl.probe<uint<1>>
) {
%dest_cast = firrtl.ref.cast %r : (!firrtl.probe<uint<1>>) -> !firrtl.probe<uint>
%x = firrtl.wire : !firrtl.uint
%xref = firrtl.ref.send %x : !firrtl.uint
// expected-error @below {{has invalid flow: the destination expression has source flow, expected sink or duplex flow}}
// expected-error @below {{has invalid flow: the destination expression "r" has source flow, expected sink or duplex flow}}
firrtl.ref.define %dest_cast, %xref : !firrtl.probe<uint>
}
}
Expand Down