Skip to content

Commit 4e3d608

Browse files
swift-ciswift-ci
authored
Merge pull request #63921 from meg-gupta/movevalueossaonly
Fix SILInliner to create move_value in OSSA only
2 parents 259b4c3 + e5eeb26 commit 4e3d608

File tree

11 files changed

+73
-89
lines changed

11 files changed

+73
-89
lines changed

‎include/swift/SIL/SILBuilder.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,6 +1278,7 @@ class SILBuilder {
12781278
}
12791279

12801280
CopyValueInst *createCopyValue(SILLocation Loc, SILValue operand) {
1281+
assert(getFunction().hasOwnership());
12811282
assert(!operand->getType().isTrivial(getFunction()) &&
12821283
"Should not be passing trivial values to this api. Use instead "
12831284
"emitCopyValueOperation");
@@ -1296,6 +1297,7 @@ class SILBuilder {
12961297

12971298
DestroyValueInst *createDestroyValue(SILLocation Loc, SILValue operand,
12981299
bool poisonRefs = false) {
1300+
assert(getFunction().hasOwnership());
12991301
assert(isLoadableOrOpaque(operand->getType()));
13001302
assert(!operand->getType().isTrivial(getFunction()) &&
13011303
"Should not be passing trivial values to this api. Use instead "
@@ -1306,6 +1308,7 @@ class SILBuilder {
13061308

13071309
MoveValueInst *createMoveValue(SILLocation loc, SILValue operand,
13081310
bool isLexical = false) {
1311+
assert(getFunction().hasOwnership());
13091312
assert(!operand->getType().isTrivial(getFunction()) &&
13101313
"Should not be passing trivial values to this api. Use instead "
13111314
"emitMoveValueOperation");

‎include/swift/SIL/SILCloner.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1870,6 +1870,9 @@ void SILCloner<ImplClass>::visitExplicitCopyValueInst(
18701870
template <typename ImplClass>
18711871
void SILCloner<ImplClass>::visitMoveValueInst(MoveValueInst *Inst) {
18721872
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
1873+
if (!getBuilder().hasOwnership()) {
1874+
return recordFoldedValue(Inst, getOpValue(Inst->getOperand()));
1875+
}
18731876
auto *MVI = getBuilder().createMoveValue(getOpLocation(Inst->getLoc()),
18741877
getOpValue(Inst->getOperand()),
18751878
Inst->isLexical());

‎lib/SILOptimizer/Utils/SILInliner.cpp

Lines changed: 49 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -449,50 +449,58 @@ void SILInlineCloner::cloneInline(ArrayRef<SILValue> AppliedArgs) {
449449

450450
SmallVector<SILValue, 4> entryArgs;
451451
entryArgs.reserve(AppliedArgs.size());
452+
453+
auto calleeConv = getCalleeFunction()->getConventions();
452454
SmallBitVector borrowedArgs(AppliedArgs.size());
453455
SmallBitVector copiedArgs(AppliedArgs.size());
456+
if (!Apply->getFunction()->hasOwnership()) {
454457

455-
auto calleeConv = getCalleeFunction()->getConventions();
456-
for (auto p : llvm::enumerate(AppliedArgs)) {
457-
SILValue callArg = p.value();
458-
SWIFT_DEFER { entryArgs.push_back(callArg); };
459-
unsigned idx = p.index();
460-
if (idx >= calleeConv.getSILArgIndexOfFirstParam()) {
461-
auto paramInfo = calleeConv.getParamInfoForSILArg(idx);
462-
if (callArg->getType().isAddress()) {
463-
// If lexical lifetimes are enabled, any alloc_stacks in the caller that
464-
// are passed to the callee being inlined (except mutating exclusive
465-
// accesses) need to be promoted to be lexical. Otherwise,
466-
// destroy_addrs could be hoisted through the body of the newly inlined
467-
// function without regard to the deinit barriers it contains.
468-
//
469-
// TODO: [begin_borrow_addr] Instead of marking the alloc_stack as a
470-
// whole lexical, just mark the inlined range lexical via
471-
// begin_borrow_addr [lexical]/end_borrow_addr just as is done
472-
// with values.
473-
auto &module = Apply.getFunction()->getModule();
474-
auto enableLexicalLifetimes =
475-
module.getASTContext().SILOpts.supportsLexicalLifetimes(module);
476-
if (!enableLexicalLifetimes)
477-
continue;
478-
479-
// Exclusive mutating accesses don't entail a lexical scope.
480-
if (paramInfo.getConvention() == ParameterConvention::Indirect_Inout)
481-
continue;
482-
483-
auto storage = AccessStorageWithBase::compute(callArg);
484-
if (auto *asi = dyn_cast_or_null<AllocStackInst>(storage.base))
485-
asi->setIsLexical();
486-
} else {
487-
// Insert begin/end borrow for guaranteed arguments.
488-
if (paramInfo.isGuaranteed()) {
489-
if (SILValue newValue = borrowFunctionArgument(callArg, idx)) {
490-
callArg = newValue;
491-
borrowedArgs[idx] = true;
492-
}
493-
} else if (paramInfo.isConsumed()) {
494-
if (SILValue newValue = moveFunctionArgument(callArg, idx)) {
495-
callArg = newValue;
458+
for (auto p : llvm::enumerate(AppliedArgs)) {
459+
SILValue callArg = p.value();
460+
entryArgs.push_back(callArg);
461+
}
462+
} else {
463+
for (auto p : llvm::enumerate(AppliedArgs)) {
464+
SILValue callArg = p.value();
465+
SWIFT_DEFER { entryArgs.push_back(callArg); };
466+
unsigned idx = p.index();
467+
if (idx >= calleeConv.getSILArgIndexOfFirstParam()) {
468+
auto paramInfo = calleeConv.getParamInfoForSILArg(idx);
469+
if (callArg->getType().isAddress()) {
470+
// If lexical lifetimes are enabled, any alloc_stacks in the caller
471+
// that are passed to the callee being inlined (except mutating
472+
// exclusive accesses) need to be promoted to be lexical. Otherwise,
473+
// destroy_addrs could be hoisted through the body of the newly
474+
// inlined function without regard to the deinit barriers it contains.
475+
//
476+
// TODO: [begin_borrow_addr] Instead of marking the alloc_stack as a
477+
// whole lexical, just mark the inlined range lexical via
478+
// begin_borrow_addr [lexical]/end_borrow_addr just as is done
479+
// with values.
480+
auto &module = Apply.getFunction()->getModule();
481+
auto enableLexicalLifetimes =
482+
module.getASTContext().SILOpts.supportsLexicalLifetimes(module);
483+
if (!enableLexicalLifetimes)
484+
continue;
485+
486+
// Exclusive mutating accesses don't entail a lexical scope.
487+
if (paramInfo.getConvention() == ParameterConvention::Indirect_Inout)
488+
continue;
489+
490+
auto storage = AccessStorageWithBase::compute(callArg);
491+
if (auto *asi = dyn_cast_or_null<AllocStackInst>(storage.base))
492+
asi->setIsLexical();
493+
} else {
494+
// Insert begin/end borrow for guaranteed arguments.
495+
if (paramInfo.isGuaranteed()) {
496+
if (SILValue newValue = borrowFunctionArgument(callArg, idx)) {
497+
callArg = newValue;
498+
borrowedArgs[idx] = true;
499+
}
500+
} else if (paramInfo.isConsumed()) {
501+
if (SILValue newValue = moveFunctionArgument(callArg, idx)) {
502+
callArg = newValue;
503+
}
496504
}
497505
}
498506
}

‎test/IRGen/move_value.sil

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ import Builtin
1010
// CHECK-NEXT: entry:
1111
// CHECK-NEXT: ret
1212
// CHECK-NEXT: }
13-
sil @move_value_test : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
14-
bb0(%0 : $Builtin.NativeObject):
13+
sil [ossa] @move_value_test : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
14+
bb0(%0 : @owned $Builtin.NativeObject):
1515
%1 = move_value %0 : $Builtin.NativeObject
1616
return %1 : $Builtin.NativeObject
17-
}
17+
}
18+

‎test/Profiler/rdar39146527.sil

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,9 @@ bb9(%31 : $(Character, allowingJoined: Bool)):
7676
%35 = struct_extract %34 : $_StringGuts, #_StringGuts._object
7777
%36 = struct_extract %35 : $_StringObject, #_StringObject._object
7878
strong_retain %36 : $Builtin.BridgeObject
79-
%38 = move_value [lexical] %32 : $Character
80-
8179
// CHECK: increment_profiler_counter 2, "$s4Test4NameO11valueStringSSvg", num_counters 4, hash 0
8280
increment_profiler_counter 2, "$s4Test4NameO11valueStringSSvg", num_counters 4, hash 0
83-
%40 = struct_extract %38 : $Character, #Character._str
81+
%40 = struct_extract %32 : $Character, #Character._str
8482
release_value %25 : $Name
8583
br bb11(%40 : $String)
8684

‎test/SIL/Parser/basic2.sil

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -63,19 +63,6 @@ bb0(%0 : @owned $Builtin.NativeObject):
6363
return %5 : $Builtin.NativeObject
6464
}
6565

66-
// CHECK-LABEL: sil @test_movevalue_parsing_non_ossa : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
67-
// CHECK: bb0(%0 :
68-
// CHECK-NEXT: %1 = move_value %0 : $Builtin.NativeObject
69-
// CHECK-NEXT: %2 = move_value [allows_diagnostics] %1 : $Builtin.NativeObject
70-
// CHECK-NEXT: return
71-
// CHECK-NEXT: } // end sil function 'test_movevalue_parsing_non_ossa'
72-
sil @test_movevalue_parsing_non_ossa : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
73-
bb0(%0 : $Builtin.NativeObject):
74-
%1 = move_value %0 : $Builtin.NativeObject
75-
%2 = move_value [allows_diagnostics] %1 : $Builtin.NativeObject
76-
return %2 : $Builtin.NativeObject
77-
}
78-
7966
// CHECK-LABEL: sil [ossa] @test_debug_value_alloc_stack_moved : $@convention(thin) (@owned Builtin.NativeObject) -> () {
8067
// CHECK: debug_value [moved] %0 : $Builtin.NativeObject
8168
// CHECK: debug_value [poison] %0 : $Builtin.NativeObject
@@ -142,4 +129,4 @@ bb0(%0 : @owned $Builtin.NativeObject):
142129
dealloc_stack %1 : $*Builtin.NativeObject
143130
%9999 = tuple()
144131
return %9999 : $()
145-
}
132+
}

‎test/SIL/Serialization/basic.sil

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -87,17 +87,6 @@ bb0(%0 : @owned $Builtin.NativeObject):
8787
return %5 : $Builtin.NativeObject
8888
}
8989

90-
// CHECK-LABEL: sil @test_movevalue_parsing_non_ossa : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
91-
// CHECK: bb0(%0 :
92-
// CHECK-NEXT: %1 = move_value %0 : $Builtin.NativeObject
93-
// CHECK-NEXT: return
94-
// CHECK-NEXT: } // end sil function 'test_movevalue_parsing_non_ossa'
95-
sil @test_movevalue_parsing_non_ossa : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
96-
bb0(%0 : $Builtin.NativeObject):
97-
%1 = move_value %0 : $Builtin.NativeObject
98-
return %1 : $Builtin.NativeObject
99-
}
100-
10190
// CHECK-LABEL: sil [no_allocation] [ossa] @test_no_allocation : $@convention(thin) () -> () {
10291
sil [no_allocation] [ossa] @test_no_allocation : $@convention(thin) () -> () {
10392
bb0:

‎test/SIL/opaque-verify.sil

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,14 @@ protocol P {
77
}
88

99
// Ensure that open_existential_addr can be used by an unqualified load.
10-
sil @loadFromExis : $@convention(thin) (@in P) -> () {
11-
bb(%0 : $P):
10+
sil [ossa] @loadFromExis : $@convention(thin) (@in P) -> () {
11+
bb(%0 : @owned $P):
1212
%tempP = alloc_stack $P, var
13-
store %0 to %tempP : $*P
14-
%opened = open_existential_addr immutable_access %tempP : $*P to $*@opened("295A5344-9728-11E7-B89E-38C9860EC692", P) Self
15-
%val = load %opened : $*@opened("295A5344-9728-11E7-B89E-38C9860EC692", P) Self
16-
%copy = copy_value %val : $@opened("295A5344-9728-11E7-B89E-38C9860EC692", P) Self
13+
store %0 to [init] %tempP : $*P
14+
%opened = open_existential_addr mutable_access %tempP : $*P to $*@opened("295A5344-9728-11E7-B89E-38C9860EC692", P) Self
15+
%val = load [take] %opened : $*@opened("295A5344-9728-11E7-B89E-38C9860EC692", P) Self
1716
%wm = witness_method $@opened("295A5344-9728-11E7-B89E-38C9860EC692", P) Self, #P.foo : <Self where Self : P> (Self) -> () -> (), %opened : $*@opened("295A5344-9728-11E7-B89E-38C9860EC692", P) Self : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> ()
18-
%call = apply %wm<@opened("295A5344-9728-11E7-B89E-38C9860EC692", P) Self>(%copy) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> ()
17+
%call = apply %wm<@opened("295A5344-9728-11E7-B89E-38C9860EC692", P) Self>(%val) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> ()
1918
destroy_value %val : $@opened("295A5344-9728-11E7-B89E-38C9860EC692", P) Self
2019
dealloc_stack %tempP : $*P
2120
%void = tuple ()

‎test/SILOptimizer/funcsig_opaque.sil

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,16 @@ sil @testAggregateArgHelper : $@convention(thin) (@owned R<S>) -> ()
2424
//
2525
// CHECK-LABEL: sil shared @$s16testAggregateArgTf4g_n : $@convention(thin) (@in_guaranteed R<S>) -> @out () {
2626
// CHECK: bb0(%0 : $R<S>):
27-
// CHECK: [[COPY:%.*]] = copy_value %0 : $R<S>
2827
// CHECK: retain_value %0 : $R<S>
2928
// CHECK: [[F:%.*]] = function_ref @testAggregateArgHelper : $@convention(thin) (@owned R<S>) -> ()
30-
// CHECK: [[CALL:%.*]] = apply [[F]]([[COPY]]) : $@convention(thin) (@owned R<S>) -> ()
31-
// CHECK: destroy_value %0 : $R<S>
29+
// CHECK: [[CALL:%.*]] = apply [[F]](%0) : $@convention(thin) (@owned R<S>) -> ()
3230
// CHECK: return %{{.*}} : $()
3331
// CHECK-LABEL: } // end sil function '$s16testAggregateArgTf4g_n'
3432
sil @testAggregateArg : $@convention(thin) (@in R<S>) -> @out () {
3533
bb0(%0 : $R<S>):
36-
%9 = copy_value %0 : $R<S>
3734
retain_value %0 : $R<S>
3835
%10 = function_ref @testAggregateArgHelper : $@convention(thin) (@owned R<S>) -> ()
39-
%12 = apply %10(%9) : $@convention(thin) (@owned R<S>) -> ()
40-
destroy_value %0 : $R<S>
36+
%12 = apply %10(%0) : $@convention(thin) (@owned R<S>) -> ()
4137
release_value %0 : $R<S>
4238
%1284 = tuple ()
4339
return %1284 : $()

‎test/SILOptimizer/specialize_dynamic_self.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ extension P {
1111

1212
class C<T> : P {
1313
// CHECK-LABEL: sil shared [noinline] @$s23specialize_dynamic_self1CC11returnsSelfACyxGXDyFSi_Tg5 : $@convention(method) (@guaranteed C<Int>) -> @owned C<Int>
14-
// CHECK: [[RESULT:%.*]] = alloc_stack [lexical] $C<Int>
14+
// CHECK: [[RESULT:%.*]] = alloc_stack $C<Int>
1515
// CHECK: [[FN:%.*]] = function_ref @$s23specialize_dynamic_self1PPAAE7method1yyF : $@convention(method) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> ()
1616
// CHECK: apply [[FN]]<@dynamic_self C<Int>>([[RESULT]]) : $@convention(method) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> ()
1717
// CHECK: return %0 : $C<Int>

‎test/SILOptimizer/specialize_opaque.sil

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,16 @@ import Builtin
66

77
// Test that foo is specialized on Builtin.Int64 and the copy_values and destroy_values are dropped.
88
//
9-
// CHECK-LABEL: sil shared @$s3fooBi64__Tg5 : $@convention(thin) (Builtin.Int64, Builtin.Int64) -> () {
9+
// CHECK-LABEL: sil shared [ossa] @$s3fooBi64__Tg5 : $@convention(thin) (Builtin.Int64, Builtin.Int64) -> () {
1010
// CHECK: bb0(%0 : $Builtin.Int64, %1 : $Builtin.Int64):
1111
// CHECK: [[F:%.*]] = function_ref @$s3fooBi64__Tg5 : $@convention(thin) (Builtin.Int64, Builtin.Int64) -> ()
1212
// CHECK: %{{.*}} = apply [[F]](%0, %1) : $@convention(thin) (Builtin.Int64, Builtin.Int64) -> ()
1313
// CHECK-NOT: copy_value
1414
// CHECK-NOT: destroy_value
1515
// CHECK: return %{{.*}} : $()
1616
// CHECK-LABEL: } // end sil function '$s3fooBi64__Tg5'
17-
sil hidden @foo : $@convention(thin) <T> (@in T, @in T) -> () {
18-
bb0(%0 : $T, %1 : $T):
17+
sil hidden [ossa] @foo : $@convention(thin) <T> (@in T, @in T) -> () {
18+
bb0(%0 : @owned $T, %1 : @owned $T):
1919
%f = function_ref @foo : $@convention(thin) <τ_0_0> (@in τ_0_0, @in τ_0_0) -> ()
2020
%cp0 = copy_value %0 : $T
2121
%cp1 = copy_value %1 : $T

0 commit comments

Comments
 (0)