Skip to content

Commit b0676be

Browse files
committed
[allocbox-to-stack] Fix an ossa bug in PromotedParamCloner.
For those who are unfamiliar, alloc-box-to-stack while generally not interprocedural, will look one level into the callgraph to see if a partial_apply that captures a box really needs to capture the box due to an escape. If not, allocbox-to-stack clones the closure with the address inside the box being passed instead of the box itself. This can then allow us to promote the box from the heap to the stack. What went wrong here is that in OSSA, this promoted param cloner drops copy_value, destroy_value, and project_box on the given box. Both the copy_value and destroy_value cases correctly looked through copy_values, but when porting, the author forgot to handle project_box as well. This then caused the cloner to assert since: 1. The project_box in the original function had a copy_value operand. 2. When we visited that copy_value, we saw it was for the box, so we dropped the copy_value and did not add it to the cloner's Value -> op(Value) map. 3. Then when the cloner tried to create op(project_box), it tries to lookup the value associated with the copy_value that is the project_box's operand... but we don't have any such value due to (2). =><=. The test change exercises this code path by adding a (project_box (copy_value)) to one of the allocbox to stack tests.
1 parent 887464b commit b0676be

File tree

2 files changed

+23
-13
lines changed

2 files changed

+23
-13
lines changed

lib/SILOptimizer/Transforms/AllocBoxToStack.cpp

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -818,28 +818,33 @@ PromotedParamCloner::visitStrongRetainInst(StrongRetainInst *Inst) {
818818
SILCloner<PromotedParamCloner>::visitStrongRetainInst(Inst);
819819
}
820820

821-
void PromotedParamCloner::visitCopyValueInst(CopyValueInst *CVI) {
821+
void PromotedParamCloner::visitCopyValueInst(CopyValueInst *cvi) {
822822
// If it's a copy of a promoted parameter, just drop the instruction.
823-
auto *Tmp = CVI;
824-
while (auto *CopyOp = dyn_cast<CopyValueInst>(Tmp->getOperand())) {
825-
Tmp = CopyOp;
823+
auto *tmp = cvi;
824+
while (auto *copyOp = dyn_cast<CopyValueInst>(tmp->getOperand())) {
825+
tmp = copyOp;
826826
}
827-
if (OrigPromotedParameters.count(Tmp->getOperand()))
827+
if (OrigPromotedParameters.count(tmp->getOperand()))
828828
return;
829829

830-
SILCloner<PromotedParamCloner>::visitCopyValueInst(CVI);
830+
SILCloner<PromotedParamCloner>::visitCopyValueInst(cvi);
831831
}
832832

833-
void PromotedParamCloner::visitProjectBoxInst(ProjectBoxInst *Inst) {
834-
// If it's a projection of a promoted parameter, drop the instruction.
835-
// Its uses will be replaced by the promoted address.
836-
if (OrigPromotedParameters.count(Inst->getOperand())) {
837-
auto *origArg = cast<SILFunctionArgument>(Inst->getOperand());
838-
recordFoldedValue(Inst, NewPromotedArgs[origArg->getIndex()]);
833+
void PromotedParamCloner::visitProjectBoxInst(ProjectBoxInst *pbi) {
834+
// If it's a projection of a promoted parameter (or a copy_value of a promoted
835+
// parameter), drop the instruction. Its uses will be replaced by the
836+
// promoted address.
837+
SILValue box = pbi->getOperand();
838+
while (auto *copyOp = dyn_cast<CopyValueInst>(box)) {
839+
box = copyOp->getOperand();
840+
}
841+
if (OrigPromotedParameters.count(box)) {
842+
auto *origArg = cast<SILFunctionArgument>(box);
843+
recordFoldedValue(pbi, NewPromotedArgs[origArg->getIndex()]);
839844
return;
840845
}
841846

842-
SILCloner<PromotedParamCloner>::visitProjectBoxInst(Inst);
847+
SILCloner<PromotedParamCloner>::visitProjectBoxInst(pbi);
843848
}
844849

845850
// While cloning during specialization, make sure apply instructions do not have

test/SILOptimizer/allocbox_to_stack_ownership.sil

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,11 @@ bb0(%0 : $Int, %1 : @owned $<τ_0_0 where τ_0_0 : Count> { var S<τ_0_0> } <T>)
680680
%6 = function_ref @get : $@convention(method) <τ_0_0 where τ_0_0 : Count> (@in S<τ_0_0>) -> Int
681681
%7 = apply %6<T>(%4) : $@convention(method) <τ_0_0 where τ_0_0 : Count> (@in S<τ_0_0>) -> Int
682682
%8 = apply %3(%0, %7) : $@convention(thin) (Int, Int) -> Bool
683+
%9 = copy_value %1 : $<τ_0_0 where τ_0_0 : Count> { var S<τ_0_0> } <T>
684+
%10 = project_box %9 : $<τ_0_0 where τ_0_0 : Count> { var S<τ_0_0> } <T>, 0
685+
copy_addr %10 to [initialization] %4 : $*S<T>
686+
destroy_value %9 : $<τ_0_0 where τ_0_0 : Count> { var S<τ_0_0> } <T>
687+
destroy_addr %4 : $*S<T>
683688
dealloc_stack %4 : $*S<T>
684689
destroy_value %1 : $<τ_0_0 where τ_0_0 : Count> { var S<τ_0_0> } <T>
685690
// CHECK: return

0 commit comments

Comments
 (0)