Skip to content

Commit a8c5d4e

Browse files
committed
[PerformanceInliner] Stack nest at OSSA lowering.
Now that in OSSA `partial_apply [on_stack]`s are represented as owned values rather than stack locations, it is possible for their destroys to violate stack discipline. A direct lowering of the instructions to non-OSSA would violate stack nesting. Previously, when inlining, it was assumed that non-coroutine callees maintained stack discipline. And, when inlining an OSSA function into a non-OSSA function, OSSA instructions were lowered directly. The result was that stack discipline could be violated. Here, when inlining a function in OSSA form into a function lowered out of OSSA form, stack nesting is fixed up.
1 parent 32685ce commit a8c5d4e

File tree

2 files changed

+42
-0
lines changed

2 files changed

+42
-0
lines changed

lib/SILOptimizer/Transforms/PerformanceInliner.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,6 +1040,12 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller) {
10401040
// will assert, so we are safe making this assumption.
10411041
SILInliner::inlineFullApply(AI, SILInliner::InlineKind::PerformanceInline,
10421042
FuncBuilder, deleter);
1043+
// When inlining an OSSA function into a non-OSSA function, ownership of
1044+
// nonescaping closures is lowered. At that point, they are recognized as
1045+
// stack users. Since they weren't recognized as such before, they may not
1046+
// satisfy stack discipline. Fix that up now.
1047+
invalidatedStackNesting |=
1048+
Callee->hasOwnership() && !Caller->hasOwnership();
10431049
++NumFunctionsInlined;
10441050
if (SILPrintInliningCallerAfter) {
10451051
printInliningDetailsCallerAfter(PassName, Caller, Callee);
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all %s -early-inline | %FileCheck %s
2+
3+
import Builtin
4+
5+
sil @paable : $@convention(thin) (Builtin.Int64) -> ()
6+
7+
sil [ossa] @partial_apply_on_stack_nesting_violator : $@convention(thin) () -> () {
8+
%paable = function_ref @paable : $@convention(thin) (Builtin.Int64) -> ()
9+
%one = integer_literal $Builtin.Int64, 1
10+
%first = partial_apply [callee_guaranteed] [on_stack] %paable(%one) : $@convention(thin) (Builtin.Int64) -> ()
11+
%two = integer_literal $Builtin.Int64, 2
12+
%second = partial_apply [callee_guaranteed] [on_stack] %paable(%two) : $@convention(thin) (Builtin.Int64) -> ()
13+
// Note that the destroy_values do not occur in an order which coincides
14+
// with stack disciplined dealloc_stacks.
15+
destroy_value %first : $@noescape @callee_guaranteed () -> ()
16+
destroy_value %second : $@noescape @callee_guaranteed () -> ()
17+
%retval = tuple ()
18+
return %retval : $()
19+
}
20+
21+
// Verify that when inlining partial_apply_on_stack_nesting_violator, the stack
22+
// nesting of the on_stack closures is fixed.
23+
// CHECK-LABEL: sil @test_inline_stack_violating_ossa_func : {{.*}} {
24+
// CHECK: [[PAABLE:%[^,]+]] = function_ref @paable
25+
// CHECK: [[FIRST:%[^,]+]] = partial_apply [callee_guaranteed] [on_stack] [[PAABLE]]
26+
// CHECK: [[SECOND:%[^,]+]] = partial_apply [callee_guaranteed] [on_stack] [[PAABLE]]
27+
// CHECK: dealloc_stack [[SECOND]]
28+
// CHECK: dealloc_stack [[FIRST]]
29+
// CHECK-LABEL: } // end sil function 'test_inline_stack_violating_ossa_func'
30+
sil @test_inline_stack_violating_ossa_func : $@convention(thin) () -> () {
31+
%callee = function_ref @partial_apply_on_stack_nesting_violator : $@convention(thin) () -> ()
32+
apply %callee() : $@convention(thin) () -> ()
33+
%retval = tuple ()
34+
return %retval : $()
35+
}
36+

0 commit comments

Comments
 (0)