Skip to content

Commit

Permalink
[FIRRTL] Disallow reading from property ports (#8191)
Browse files Browse the repository at this point in the history
According to the FIRRTL spec, the right hand side of a propassign must
have source flow.  This means that it is illegal to read from a property
output port, something that is allowed for regular HW ports.  This
change enforces this property during flow checking in the operation
verifiers.
  • Loading branch information
youngar authored Feb 5, 2025
1 parent 638624d commit 2d97537
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
6 changes: 4 additions & 2 deletions lib/Dialect/FIRRTL/FIRRTLOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3518,9 +3518,11 @@ static LogicalResult checkConnectFlow(Operation *connect) {
// instance/memory input ports.
auto srcFlow = foldFlow(src);
if (!isValidSrc(srcFlow)) {
// A sink that is a port output or instance input used as a source is okay.
// A sink that is a port output or instance input used as a source is okay,
// as long as it is not a property.
auto kind = getDeclarationKind(src);
if (kind != DeclKind::Port && kind != DeclKind::Instance) {
if (isa<PropertyType>(src.getType()) ||
(kind != DeclKind::Port && kind != DeclKind::Instance)) {
auto srcRef = getFieldRefFromValue(src, /*lookThroughCasts=*/true);
auto [srcName, rootKnown] = getFieldName(srcRef);
auto diag = emitError(connect->getLoc());
Expand Down
23 changes: 23 additions & 0 deletions test/Dialect/FIRRTL/errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -2199,6 +2199,29 @@ firrtl.circuit "Top" {
}
}

// -----
// Try to read from an output property port with sink flow.
firrtl.circuit "Top" {
// expected-note @below {{the source was defined here}}
firrtl.module @Top(out %a : !firrtl.string, out %b : !firrtl.string) {
// expected-error @below {{connect has invalid flow: the source expression "b" has sink flow, expected source or duplex flow}}
firrtl.propassign %a, %b : !firrtl.string
}
}

// -----
// Try to read from an input property instance port with sink flow.

firrtl.circuit "Top" {
firrtl.module @Child(in %in : !firrtl.string) { }
firrtl.module @Top(out %out : !firrtl.string) {
// expected-note @below {{the source was defined here}}
%child_in = firrtl.instance child @Child(in in : !firrtl.string)
// expected-error @below {{connect has invalid flow: the source expression "child.in" has sink flow, expected source or duplex flow}}
firrtl.propassign %out, %child_in : !firrtl.string
}
}

// -----
// Try to assign to the input port of an output object of a local object.
// This fails because we can only assign directly to the ports of a local object
Expand Down

0 comments on commit 2d97537

Please sign in to comment.