Skip to content

Commit a9ca793

Browse files
committed
[allocbox-to-stack] Eliminate temporary dominance issue and fix improper use of non-ossa generic method createDestroyValue.
NOTE: I also added a partial_apply [guaranteed] test. Whats interesting about these is that we only ever perform allocbox_to_stack if we know that we are going to eliminate the allocbox completely. So if we break dominance among some uses of the alloc box or insert destroy_value when we are in non-ossa... it doesn't matter since we will eliminate the box and these uses before the pass is done running. This will harmless on the surface is an instance of the compiler being in a "fixed point of correctness". This occurance is when the compiler implementation is incorrect but the incorrectness is being hidden in the final output. If the output of the compiler changes or the code in question is changed, new bugs can be introduced due to the lack of preserving of standard invariants like dominance. I also added an additional helper: SILBuilder::insertAfter(SILValue). This builds on Erik's commit that gave us insert(SILInstruction *). I wanted this functionality, but additionally I wanted to make it so that if I had an argument, I got back the first instruction in the block. So it was natural to extend this to values.
1 parent beeefbc commit a9ca793

File tree

4 files changed

+109
-8
lines changed

4 files changed

+109
-8
lines changed

include/swift/SIL/SILBuilder.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2527,6 +2527,21 @@ class SILBuilderWithScope : public SILBuilder {
25272527
/// predecessor block: the parent of \p inst.
25282528
static void insertAfter(SILInstruction *inst,
25292529
function_ref<void(SILBuilder &)> func);
2530+
2531+
/// If \p is an inst, then this is equivalent to insertAfter(inst). If a
2532+
/// SILArgument is passed in, we use the first instruction in its parent
2533+
/// block. We assert on undef.
2534+
static void insertAfter(SILValue value,
2535+
function_ref<void(SILBuilder &)> func) {
2536+
if (auto *i = dyn_cast<SingleValueInstruction>(value))
2537+
return insertAfter(i, func);
2538+
if (auto *mvir = dyn_cast<MultipleValueInstructionResult>(value))
2539+
return insertAfter(mvir->getParent(), func);
2540+
if (auto *arg = dyn_cast<SILArgument>(value))
2541+
return insertAfter(&*arg->getParent()->begin(), func);
2542+
assert(!isa<SILUndef>(value) && "This API can not use undef");
2543+
llvm_unreachable("Unhandled case?!");
2544+
}
25302545
};
25312546

25322547
class SavedInsertionPointRAII {

lib/SILOptimizer/Transforms/AllocBoxToStack.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -920,16 +920,15 @@ specializeApplySite(SILOptFunctionBuilder &FuncBuilder, ApplySite Apply,
920920
continue;
921921
}
922922

923-
auto Box = O.get();
924-
assert(((isa<SingleValueInstruction>(Box) && isa<AllocBoxInst>(Box) ||
925-
isa<CopyValueInst>(Box)) ||
923+
SILValue Box = O.get();
924+
assert((isa<SingleValueInstruction>(Box) && isa<AllocBoxInst>(Box) ||
925+
isa<CopyValueInst>(Box) ||
926926
isa<SILFunctionArgument>(Box)) &&
927927
"Expected either an alloc box or a copy of an alloc box or a "
928928
"function argument");
929-
auto InsertPt = Box->getDefiningInsertionPoint();
930-
assert(InsertPt);
931-
SILBuilderWithScope B(InsertPt);
932-
Args.push_back(B.createProjectBox(Box.getLoc(), Box, 0));
929+
SILBuilderWithScope::insertAfter(Box, [&](SILBuilder &B) {
930+
Args.push_back(B.createProjectBox(Box.getLoc(), Box, 0));
931+
});
933932

934933
// For a partial_apply, if this argument is promoted, it is a box that we're
935934
// turning into an address because we've proven we can keep this value on
@@ -950,7 +949,7 @@ specializeApplySite(SILOptFunctionBuilder &FuncBuilder, ApplySite Apply,
950949
// becomes dead.
951950
for (SILInstruction *FrontierInst : PAFrontier) {
952951
SILBuilderWithScope Builder(FrontierInst);
953-
Builder.createDestroyValue(Apply.getLoc(), Box);
952+
Builder.emitDestroyValueOperation(Apply.getLoc(), Box);
954953
}
955954
}
956955
}

test/SILOptimizer/allocbox_to_stack.sil

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,18 @@ bb0(%0 : $@callee_owned () -> @out U):
486486
return %8 : $()
487487
}
488488

489+
sil [transparent] [serialized] @mightApplyGuaranteed : $@convention(thin) <U where U : P> (@guaranteed @callee_guaranteed () -> @out U) -> () {
490+
bb0(%0 : $@callee_guaranteed () -> @out U):
491+
debug_value %0 : $@callee_guaranteed () -> @out U
492+
%3 = alloc_stack $U
493+
%4 = apply %0(%3) : $@callee_guaranteed () -> @out U
494+
destroy_addr %3 : $*U
495+
dealloc_stack %3 : $*U
496+
%8 = tuple ()
497+
return %8 : $()
498+
}
499+
500+
489501
// CHECK-LABEL: sil @callWithAutoclosure
490502
sil @callWithAutoclosure : $@convention(thin) <T where T : P> (@in T) -> () {
491503
// CHECK: bb0
@@ -513,6 +525,37 @@ bb0(%0 : $*T):
513525
return %9 : $()
514526
}
515527

528+
529+
// CHECK-LABEL: sil @callWithAutoclosure_2 : $@convention(thin) <T where T : P> (@in T) -> () {
530+
sil @callWithAutoclosure_2 : $@convention(thin) <T where T : P> (@in T) -> () {
531+
// CHECK: bb0
532+
bb0(%0 : $*T):
533+
// CHECK: debug_value_addr
534+
debug_value_addr %0 : $*T
535+
// CHECK: function_ref @mightApply
536+
%2 = function_ref @mightApply : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@owned @callee_owned () -> @out τ_0_0) -> ()
537+
%3 = function_ref @closure_to_specialize : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@owned <τ_0_0> { var τ_0_0 } <τ_0_0>) -> @out τ_0_0
538+
// CHECK-NOT: alloc_box
539+
// CHECK: [[STACK:%[0-9a-zA-Z_]+]] = alloc_stack $T
540+
%4 = alloc_box $<τ_0_0> { var τ_0_0 } <T>
541+
%4a = project_box %4 : $<τ_0_0> { var τ_0_0 } <T>, 0
542+
strong_retain %4 : $<τ_0_0> { var τ_0_0 } <T>
543+
// CHECK: copy_addr %0 to [initialization] [[STACK]] : $*T
544+
copy_addr %0 to [initialization] %4a : $*T
545+
// CHECK: [[CLOSURE:%[0-9a-zA-Z]+]] = function_ref @$s21closure_to_specializeTf0ns_n
546+
// CHECK: partial_apply [[CLOSURE]]<T>([[STACK]])
547+
%6 = partial_apply %3<T>(%4) : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@owned <τ_0_0> { var τ_0_0 } <τ_0_0>) -> @out τ_0_0
548+
%7 = apply %2<T>(%6) : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@owned @callee_owned () -> @out τ_0_0) -> ()
549+
// CHECK: destroy_addr [[STACK]] : $*T
550+
// CHECK: dealloc_stack [[STACK]] : $*T
551+
destroy_addr %0 : $*T
552+
strong_release %4 : $<τ_0_0> { var τ_0_0 } <T>
553+
%9 = tuple ()
554+
// CHECK: return
555+
return %9 : $()
556+
}
557+
// CHECK: } // end sil function 'callWithAutoclosure_2'
558+
516559
// CHECK-LABEL: sil private @$s21closure_to_specializeTf0ns_n : $@convention(thin) <T where T : P> (@inout_aliasable T) -> @out T
517560
sil private @closure_to_specialize : $@convention(thin) <T where T : P> (@owned <τ_0_0> { var τ_0_0 } <T>) -> @out T {
518561
// CHECK: bb0

test/SILOptimizer/allocbox_to_stack_ownership.sil

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,20 @@ bb0(%0 : @owned $@callee_owned () -> @out U):
483483
return %8 : $()
484484
}
485485

486+
sil [transparent] [serialized] [ossa] @mightApplyGuaranteed : $@convention(thin) <U where U : P> (@guaranteed @callee_guaranteed () -> @out U) -> () {
487+
// CHECK: bb0
488+
bb0(%0 : @guaranteed $@callee_guaranteed () -> @out U):
489+
debug_value %0 : $@callee_guaranteed () -> @out U
490+
%3 = alloc_stack $U
491+
%4 = apply %0(%3) : $@callee_guaranteed () -> @out U
492+
destroy_addr %3 : $*U
493+
dealloc_stack %3 : $*U
494+
%8 = tuple ()
495+
// CHECK: return
496+
return %8 : $()
497+
}
498+
499+
486500
// CHECK-LABEL: sil [ossa] @callWithAutoclosure
487501
sil [ossa] @callWithAutoclosure : $@convention(thin) <T where T : P> (@in T) -> () {
488502
// CHECK: bb0
@@ -510,6 +524,36 @@ bb0(%0 : $*T):
510524
return %9 : $()
511525
}
512526

527+
// CHECK-LABEL: sil [ossa] @callWithAutoclosure_2 : $@convention(thin) <T where T : P> (@in T) -> () {
528+
sil [ossa] @callWithAutoclosure_2 : $@convention(thin) <T where T : P> (@in T) -> () {
529+
// CHECK: bb0
530+
bb0(%0 : $*T):
531+
// CHECK: debug_value_addr
532+
debug_value_addr %0 : $*T
533+
// CHECK: function_ref @mightApply
534+
%2 = function_ref @mightApply : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@owned @callee_owned () -> @out τ_0_0) -> ()
535+
%3 = function_ref @closure_to_specialize : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@owned <τ_0_0> { var τ_0_0 } <τ_0_0>) -> @out τ_0_0
536+
// CHECK-NOT: alloc_box
537+
// CHECK: [[STACK:%[0-9a-zA-Z_]+]] = alloc_stack $T
538+
%4 = alloc_box $<τ_0_0> { var τ_0_0 } <T>
539+
%4a = project_box %4 : $<τ_0_0> { var τ_0_0 } <T>, 0
540+
%4copy = copy_value %4 : $<τ_0_0> { var τ_0_0 } <T>
541+
// CHECK: copy_addr %0 to [initialization] [[STACK]] : $*T
542+
copy_addr %0 to [initialization] %4a : $*T
543+
// CHECK: [[CLOSURE:%[0-9a-zA-Z]+]] = function_ref @$s21closure_to_specializeTf0ns_n
544+
// CHECK: partial_apply [[CLOSURE]]<T>([[STACK]])
545+
%6 = partial_apply %3<T>(%4copy) : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@owned <τ_0_0> { var τ_0_0 } <τ_0_0>) -> @out τ_0_0
546+
%7 = apply %2<T>(%6) : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@owned @callee_owned () -> @out τ_0_0) -> ()
547+
// CHECK: destroy_addr [[STACK]] : $*T
548+
// CHECK: dealloc_stack [[STACK]] : $*T
549+
destroy_addr %0 : $*T
550+
destroy_value %4 : $<τ_0_0> { var τ_0_0 } <T>
551+
%9 = tuple ()
552+
// CHECK: return
553+
return %9 : $()
554+
}
555+
// CHECK: } // end sil function 'callWithAutoclosure_2'
556+
513557
// CHECK-LABEL: sil shared [ossa] @$s21closure_to_specializeTf0ns_n : $@convention(thin) <T where T : P> (@inout_aliasable T) -> @out T
514558
sil shared [ossa] @closure_to_specialize : $@convention(thin) <T where T : P> (@owned <τ_0_0> { var τ_0_0 } <T>) -> @out T {
515559
// CHECK: bb0

0 commit comments

Comments
 (0)