Allow propassign under layerblocks (#6656)

Also, ban capturing outer properties inside a layerblock.
This commit is contained in:
Robert Young 2024-02-08 15:27:11 -05:00 committed by GitHub
parent 251eb19821
commit b91377259e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 69 additions and 11 deletions

View File

@ -278,8 +278,8 @@ def MatchOp : FIRRTLOp<"match", [SingleBlock, NoTerminator,
}];
}
def PropAssignOp : FIRRTLOp<"propassign",
[FConnectLike, SameTypeOperands, ParentOneOf<["FModuleOp", "ClassOp"]>]> {
def PropAssignOp : FIRRTLOp<"propassign", [FConnectLike, SameTypeOperands,
ParentOneOf<["FModuleOp", "ClassOp", "LayerBlockOp"]>]> {
let summary = "Assign to a sink property value.";
let description = [{
Assign an output property value. The types must match exactly.

View File

@ -6087,20 +6087,26 @@ LogicalResult LayerBlockOp::verify() {
if (getOperation()->isAncestor(definingOp))
continue;
// Capture of a non-base type, e.g., reference, is allowed.
FIRRTLBaseType type =
type_dyn_cast<FIRRTLBaseType>(operand.getType());
if (!type)
continue;
auto type = operand.getType();
// Capturing a non-passive type is illegal.
if (!type.isPassive()) {
auto diag = emitOpError()
<< "captures an operand which is not a passive type";
// Capture of a non-base type, e.g., reference, is allowed.
if (isa<PropertyType>(type)) {
auto diag = emitOpError() << "captures a property operand";
diag.attachNote(operand.getLoc()) << "operand is defined here";
diag.attachNote(op->getLoc()) << "operand is used here";
return WalkResult::interrupt();
}
// Capturing a non-passive type is illegal.
if (auto baseType = type_dyn_cast<FIRRTLBaseType>(type)) {
if (!baseType.isPassive()) {
auto diag = emitOpError()
<< "captures an operand which is not a passive type";
diag.attachNote(operand.getLoc()) << "operand is defined here";
diag.attachNote(op->getLoc()) << "operand is used here";
return WalkResult::interrupt();
}
}
}
// Ensure that the layer block does not drive any sinks outside.

View File

@ -131,3 +131,41 @@ firrtl.circuit "Top" {
} ()
}
}
// -----
// Capturing an outer property from inside a layerblock is not allowed.
// Eventually, we may like to relax this, but we need time to convince
// ourselves whether this is actually safe and can be lowered.
firrtl.circuit "Top" {
firrtl.layer @A bind {}
firrtl.extmodule @WithInputProp(in i : !firrtl.string) attributes {layers=[@A]}
firrtl.module @Top(out %o : !firrtl.probe<uint<1>>) {
// expected-note @below {{operand is defined here}}
%str = firrtl.string "whatever"
// expected-error @below {{'firrtl.layerblock' op captures a property operand}}
firrtl.layerblock @A {
%foo_in = firrtl.instance foo @WithInputProp(in i : !firrtl.string)
// expected-note @below {{operand is used here}}
firrtl.propassign %foo_in, %str : !firrtl.string
}
}
}
// -----
// Driving an outer property from inside a layerblock is not allowed.
firrtl.circuit "Top" {
firrtl.layer @A bind {}
firrtl.extmodule @WithInputProp(in i : !firrtl.string) attributes {layers=[@A]}
firrtl.module @Top(out %o : !firrtl.probe<uint<1>>) {
// expected-note @below {{operand is defined here}}
%foo_in = firrtl.instance foo @WithInputProp(in i : !firrtl.string)
// expected-error @below {{'firrtl.layerblock' op captures a property operand}}
firrtl.layerblock @A {
%str = firrtl.string "whatever"
// expected-note @below {{operand is used here}}
firrtl.propassign %foo_in, %str : !firrtl.string
}
}
}

View File

@ -187,4 +187,18 @@ firrtl.circuit "Test" {
firrtl.ref.define %0, %2 : !firrtl.probe<uint<1>, @A>
}
}
//===--------------------------------------------------------------------===//
// Properties Under Layers
//===--------------------------------------------------------------------===//
firrtl.extmodule @WithInputProp(in i : !firrtl.string)
firrtl.module @InstWithInputPropUnderLayerBlock() {
firrtl.layerblock @A {
%foo_in = firrtl.instance foo @WithInputProp(in i : !firrtl.string)
%str = firrtl.string "whatever"
firrtl.propassign %foo_in, %str : !firrtl.string
}
}
}