Skip to content

Commit dd160ce

Browse files
nate-chandlernate-chandler
authored
Merge pull request #63980 from nate-chandler/inlining/partial-apply-stack-discipline
[Inliner] Fix stack nesting when lowering OSSA.
2 parents 71b0ea6 + 49f1deb commit dd160ce

File tree

4 files changed

+115
-0
lines changed

4 files changed

+115
-0
lines changed

‎lib/SILOptimizer/Mandatory/MandatoryInlining.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ extern llvm::cl::opt<bool> SILPrintInliningCallerBefore;
5050

5151
extern llvm::cl::opt<bool> SILPrintInliningCallerAfter;
5252

53+
extern llvm::cl::opt<bool> EnableVerifyAfterEachInlining;
54+
5355
extern void printInliningDetailsCallee(StringRef passName, SILFunction *caller,
5456
SILFunction *callee);
5557

@@ -954,6 +956,14 @@ runOnFunctionRecursively(SILOptFunctionBuilder &FuncBuilder, SILFunction *F,
954956
// nextBB will point to the last inlined block
955957
SILBasicBlock *lastBB =
956958
Inliner.inlineFunction(CalleeFunction, InnerAI, FullArgs);
959+
960+
// When inlining an OSSA function into a non-OSSA function, ownership of
961+
// nonescaping closures is lowered. At that point, they are recognized
962+
// as stack users. Since they weren't recognized as such before, they
963+
// may not satisfy stack discipline. Fix that up now.
964+
invalidatedStackNesting |=
965+
(CalleeFunction->hasOwnership() && !F->hasOwnership());
966+
957967
if (SILPrintInliningCallerAfter) {
958968
printInliningDetailsCallerAfter("MandatoryInlining", F, CalleeFunction);
959969
}
@@ -972,6 +982,16 @@ runOnFunctionRecursively(SILOptFunctionBuilder &FuncBuilder, SILFunction *F,
972982
// later.
973983
changedFunctions.insert(F);
974984

985+
if (EnableVerifyAfterEachInlining) {
986+
if (invalidatedStackNesting) {
987+
StackNesting::fixNesting(F);
988+
changedFunctions.insert(F);
989+
invalidatedStackNesting = false;
990+
}
991+
992+
F->verify();
993+
}
994+
975995
// Resume inlining within nextBB, which contains only the inlined
976996
// instructions and possibly instructions in the original call block that
977997
// have not yet been visited.

‎lib/SILOptimizer/Transforms/PerformanceInliner.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ llvm::cl::opt<bool> SILPrintInliningCallerAfter(
6363
llvm::cl::desc(
6464
"Print functions into which another function has been inlined."));
6565

66+
llvm::cl::opt<bool> EnableVerifyAfterEachInlining(
67+
"sil-inline-verify-after-each-inline", llvm::cl::init(false),
68+
llvm::cl::desc(
69+
"Run sil verification after inlining each found callee apply "
70+
"site into a caller."));
71+
6672
//===----------------------------------------------------------------------===//
6773
// Printing Helpers
6874
//===----------------------------------------------------------------------===//
@@ -1034,10 +1040,30 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller) {
10341040
// will assert, so we are safe making this assumption.
10351041
SILInliner::inlineFullApply(AI, SILInliner::InlineKind::PerformanceInline,
10361042
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();
10371049
++NumFunctionsInlined;
10381050
if (SILPrintInliningCallerAfter) {
10391051
printInliningDetailsCallerAfter(PassName, Caller, Callee);
10401052
}
1053+
if (EnableVerifyAfterEachInlining) {
1054+
deleter.cleanupDeadInstructions();
1055+
1056+
// The inliner splits blocks at call sites. Re-merge trivial branches to
1057+
// reestablish a canonical CFG.
1058+
mergeBasicBlocks(Caller);
1059+
1060+
if (invalidatedStackNesting) {
1061+
StackNesting::fixNesting(Caller);
1062+
invalidatedStackNesting = false;
1063+
}
1064+
1065+
Caller->verify();
1066+
}
10411067
}
10421068
deleter.cleanupDeadInstructions();
10431069

‎test/SILOptimizer/inline_ossa_to_non_ossa.sil

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+

‎test/SILOptimizer/mandatory_inlining_ossa_to_non_ossa.sil

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,3 +456,36 @@ bb0(%0 : $Builtin.NativeObject):
456456
%9999 = tuple()
457457
return %9999 : $()
458458
}
459+
460+
sil @paable : $@convention(thin) (Builtin.Int64) -> ()
461+
462+
sil [transparent] [ossa] @partial_apply_on_stack_nesting_violator : $@convention(thin) () -> () {
463+
%paable = function_ref @paable : $@convention(thin) (Builtin.Int64) -> ()
464+
%one = integer_literal $Builtin.Int64, 1
465+
%first = partial_apply [callee_guaranteed] [on_stack] %paable(%one) : $@convention(thin) (Builtin.Int64) -> ()
466+
%two = integer_literal $Builtin.Int64, 2
467+
%second = partial_apply [callee_guaranteed] [on_stack] %paable(%two) : $@convention(thin) (Builtin.Int64) -> ()
468+
// Note that the destroy_values do not occur in an order which coincides
469+
// with stack disciplined dealloc_stacks.
470+
destroy_value %first : $@noescape @callee_guaranteed () -> ()
471+
destroy_value %second : $@noescape @callee_guaranteed () -> ()
472+
%retval = tuple ()
473+
return %retval : $()
474+
}
475+
476+
// Verify that when inlining partial_apply_on_stack_nesting_violator, the stack
477+
// nesting of the on_stack closures is fixed.
478+
// CHECK-LABEL: sil @test_inline_stack_violating_ossa_func : {{.*}} {
479+
// CHECK: [[PAABLE:%[^,]+]] = function_ref @paable
480+
// CHECK: [[FIRST:%[^,]+]] = partial_apply [callee_guaranteed] [on_stack] [[PAABLE]]
481+
// CHECK: [[SECOND:%[^,]+]] = partial_apply [callee_guaranteed] [on_stack] [[PAABLE]]
482+
// CHECK: dealloc_stack [[SECOND]]
483+
// CHECK: dealloc_stack [[FIRST]]
484+
// CHECK-LABEL: } // end sil function 'test_inline_stack_violating_ossa_func'
485+
sil @test_inline_stack_violating_ossa_func : $@convention(thin) () -> () {
486+
%callee = function_ref @partial_apply_on_stack_nesting_violator : $@convention(thin) () -> ()
487+
apply %callee() : $@convention(thin) () -> ()
488+
%retval = tuple ()
489+
return %retval : $()
490+
}
491+

0 commit comments

Comments
 (0)