fixup! [FIRRTL] Don't capture non-passives in LowerLayers

This commit is contained in:
Schuyler Eldridge 2024-08-09 18:35:14 -04:00
parent 73c053de32
commit d4e7857649
No known key found for this signature in database
GPG Key ID: 50C5E9936AAD536D
1 changed files with 38 additions and 29 deletions

View File

@ -497,37 +497,46 @@ LogicalResult LowerLayersPass::runOnModuleBody(FModuleOp moduleOp,
continue;
}
// If this operation has non-passive operands defined outside the
// layerblock, then it may result in a lowering that creates output ports
// on the module. This is illegal in the FIRRTL spec since no layerblock
// may write to values outside it. Try to fix this up if possible.
if (llvm::any_of(op.getOperands(), [&](Value a) {
auto type = type_dyn_cast<FIRRTLBaseType>(a.getType());
return type && !type.isPassive();
})) {
// If this is a subfield, subindex, or subaccess op, then try to fix the
// problem by moving the value outside the layerblock. This always
// works for subfield and subindex, but may fail for subaccess if the
// index is defined inside the layerblock.
if (isa<SubfieldOp, SubindexOp, SubaccessOp>(op)) {
if (llvm::none_of(op.getOperands(), [&](Value a) {
return isAncestorOfValueOwner(layerBlock, a);
})) {
op.moveBefore(layerBlock);
continue;
}
auto diag =
op.emitOpError()
<< "has a non-passive operand and captures a value defined "
"outside its enclosing bind-convention layerblock. The "
"'LowerLayers' pass cannot lower this as it would create an "
"output port on the resulting module.";
diag.attachNote(layerBlock.getLoc())
<< "the layerblock is defined here";
return WalkResult::interrupt();
}
// Handle subfields, subindexes, and subaccesses which are indexing into
// non-passive values. If these are kept in the module, then the module
// must create bi-directional ports. This doesn't make sense as the
// FIRRTL spec states that no layerblock may write to values outside it.
// Fix this for subfield and subindex by moving these ops outside the
// layerblock. Try to fix this for subaccess and error if the move can't
// be made because the index is defined inside the layerblock. (This case
// is exceedingly rare given that subaccesses are almost always unexepcted
// when this pass runs.)
if (isa<SubfieldOp, SubindexOp>(op)) {
auto input = op.getOperand(0);
if (!firrtl::type_cast<FIRRTLBaseType>(input.getType()).isPassive() &&
!isAncestorOfValueOwner(layerBlock, input))
op.moveBefore(layerBlock);
continue;
}
if (auto subOp = dyn_cast<SubaccessOp>(op)) {
auto input = subOp.getInput();
if (firrtl::type_cast<FIRRTLBaseType>(input.getType()).isPassive())
continue;
if (!isAncestorOfValueOwner(layerBlock, input) &&
!isAncestorOfValueOwner(layerBlock, subOp.getIndex())) {
subOp->moveBefore(layerBlock);
continue;
}
auto diag = op.emitOpError()
<< "has a non-passive operand and captures a value defined "
"outside its enclosing bind-convention layerblock. The "
"'LowerLayers' pass cannot lower this as it would "
"create an output port on the resulting module.";
diag.attachNote(layerBlock.getLoc())
<< "the layerblock is defined here";
return WalkResult::interrupt();
}
// Beyond this point, we are handling operations which capture values
// defined outside the layerblock. Whenever we see this, we need to
// create ports for the module that this layerblock will become.
if (auto refSend = dyn_cast<RefSendOp>(op)) {
auto src = refSend.getBase();
if (!isAncestorOfValueOwner(layerBlock, src))