Skip to content

Commit a5e7c3f

Browse files
committed
[sil-combine] Update LoadInst opts for OSSA and expand opts to also support LoadBorrow.
NOTE: The stdlib count/capacity propagation code is tested in an end<->end fashion in a separate Swift test. Once I flip the switch, that test will run. The code is pretty simple, so I feel relatively confident with it.
1 parent c99874b commit a5e7c3f

File tree

4 files changed

+204
-80
lines changed

4 files changed

+204
-80
lines changed

lib/SILOptimizer/SILCombiner/SILCombiner.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,12 @@ class SILCombiner :
220220
SILInstruction *visitStrongRetainInst(StrongRetainInst *SRI);
221221
SILInstruction *visitRefToRawPointerInst(RefToRawPointerInst *RRPI);
222222
SILInstruction *visitUpcastInst(UpcastInst *UCI);
223-
SILInstruction *optimizeLoadFromStringLiteral(LoadInst *LI);
223+
224+
// NOTE: The load optimized in this method is a load [trivial].
225+
SILInstruction *optimizeLoadFromStringLiteral(LoadInst *li);
226+
224227
SILInstruction *visitLoadInst(LoadInst *LI);
228+
SILInstruction *visitLoadBorrowInst(LoadBorrowInst *LI);
225229
SILInstruction *visitIndexAddrInst(IndexAddrInst *IA);
226230
bool optimizeStackAllocatedEnum(AllocStackInst *AS);
227231
SILInstruction *visitAllocStackInst(AllocStackInst *AS);

lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

Lines changed: 62 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -759,12 +759,13 @@ SILInstruction *SILCombiner::optimizeLoadFromStringLiteral(LoadInst *LI) {
759759

760760
/// Returns true if \p LI loads a zero integer from the empty Array, Dictionary
761761
/// or Set singleton.
762-
static bool isZeroLoadFromEmptyCollection(LoadInst *LI) {
762+
static bool isZeroLoadFromEmptyCollection(SingleValueInstruction *LI) {
763+
assert(isa<LoadInst>(LI) || isa<LoadBorrowInst>(LI));
763764
auto intTy = LI->getType().getAs<BuiltinIntegerType>();
764765
if (!intTy)
765766
return false;
766-
767-
SILValue addr = LI->getOperand();
767+
768+
SILValue addr = LI->getOperand(0);
768769

769770
// Find the root object of the load-address.
770771
for (;;) {
@@ -805,6 +806,8 @@ static bool isZeroLoadFromEmptyCollection(LoadInst *LI) {
805806
case ValueKind::UpcastInst:
806807
case ValueKind::RawPointerToRefInst:
807808
case ValueKind::AddressToPointerInst:
809+
case ValueKind::BeginBorrowInst:
810+
case ValueKind::CopyValueInst:
808811
case ValueKind::EndCOWMutationInst:
809812
addr = cast<SingleValueInstruction>(addr)->getOperand(0);
810813
break;
@@ -839,15 +842,56 @@ static SingleValueInstruction *getValueFromStaticLet(SILValue v) {
839842
return nullptr;
840843
}
841844

842-
SILInstruction *SILCombiner::visitLoadInst(LoadInst *LI) {
843-
if (LI->getFunction()->hasOwnership())
845+
SILInstruction *SILCombiner::visitLoadBorrowInst(LoadBorrowInst *lbi) {
846+
// (load (upcast-ptr %x)) -> (upcast-ref (load %x))
847+
Builder.setCurrentDebugScope(lbi->getDebugScope());
848+
if (auto *ui = dyn_cast<UpcastInst>(lbi->getOperand())) {
849+
// We want to RAUW the current load_borrow with the upcast. To do that
850+
// safely, we need to insert new end_borrow on the new load_borrow, erase
851+
// the end_borrow and then RAUW.
852+
SmallVector<EndBorrowInst *, 32> endBorrowInst;
853+
for (auto *ebi : lbi->getEndBorrows())
854+
endBorrowInst.push_back(ebi);
855+
auto newLBI = Builder.createLoadBorrow(lbi->getLoc(), ui->getOperand());
856+
for (auto *ebi : endBorrowInst) {
857+
SILBuilderWithScope builder(ebi, Builder);
858+
builder.emitEndBorrowOperation(ebi->getLoc(), newLBI);
859+
eraseInstFromFunction(*ebi);
860+
}
861+
auto *uci = Builder.createUpcast(lbi->getLoc(), newLBI, lbi->getType());
862+
replaceInstUsesWith(*lbi, uci);
863+
return eraseInstFromFunction(*lbi);
864+
}
865+
866+
// Constant-propagate the 0 value when loading "count" or "capacity" from the
867+
// empty Array, Set or Dictionary storage.
868+
// On high-level SIL this optimization is also done by the
869+
// ArrayCountPropagation pass, but only for Array. And even for Array it's
870+
// sometimes needed to propagate the empty-array count when high-level
871+
// semantics function are already inlined.
872+
// Note that for non-empty arrays/sets/dictionaries, the count can be
873+
// propagated by redundant load elimination.
874+
if (isZeroLoadFromEmptyCollection(lbi))
875+
return Builder.createIntegerLiteral(lbi->getLoc(), lbi->getType(), 0);
876+
877+
// If we have a load_borrow that only has non_debug end_borrow uses, delete
878+
// it.
879+
if (llvm::all_of(getNonDebugUses(lbi), [](Operand *use) {
880+
return isa<EndBorrowInst>(use->getUser());
881+
})) {
882+
eraseInstIncludingUsers(lbi);
844883
return nullptr;
884+
}
885+
886+
return nullptr;
887+
}
845888

889+
SILInstruction *SILCombiner::visitLoadInst(LoadInst *LI) {
846890
// (load (upcast-ptr %x)) -> (upcast-ref (load %x))
847891
Builder.setCurrentDebugScope(LI->getDebugScope());
848892
if (auto *UI = dyn_cast<UpcastInst>(LI->getOperand())) {
849-
auto NewLI = Builder.createLoad(LI->getLoc(), UI->getOperand(),
850-
LoadOwnershipQualifier::Unqualified);
893+
auto NewLI = Builder.emitLoadValueOperation(LI->getLoc(), UI->getOperand(),
894+
LI->getOwnershipQualifier());
851895
return Builder.createUpcast(LI->getLoc(), NewLI, LI->getType());
852896
}
853897

@@ -874,6 +918,17 @@ SILInstruction *SILCombiner::visitLoadInst(LoadInst *LI) {
874918
return cloner.clone(initVal);
875919
}
876920

921+
// If we have a load [copy] whose only non-debug users are destroy_value, just
922+
// eliminate it.
923+
if (LI->getOwnershipQualifier() == LoadOwnershipQualifier::Copy) {
924+
if (llvm::all_of(getNonDebugUses(LI), [](Operand *use) {
925+
return isa<DestroyValueInst>(use->getUser());
926+
})) {
927+
eraseInstIncludingUsers(LI);
928+
return nullptr;
929+
}
930+
}
931+
877932
return nullptr;
878933
}
879934

test/SILOptimizer/sil_combine_misc_opts.sil

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,18 @@ sil_stage canonical
66

77
import Builtin
88

9+
//////////////////
10+
// Declarations //
11+
//////////////////
12+
913
class Klass {}
1014

15+
sil @nativeobject_guaranteed_use : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
16+
17+
///////////
18+
// Tests //
19+
///////////
20+
1121
// We test both the ossa and non-ossa variants.
1222
//
1323
// CHECK-LABEL: sil [ossa] @fix_lifetime_promotion_ossa : $@convention(thin) (@owned Klass) -> () {
@@ -136,3 +146,39 @@ bb0:
136146
%3 = tuple ()
137147
return %3 : $()
138148
}
149+
150+
// We should have a single load [copy] here.
151+
//
152+
// CHECK-LABEL: sil [ossa] @remove_loadcopy_with_only_destroy_users : $@convention(thin) (@in_guaranteed Builtin.NativeObject) -> () {
153+
// CHECK: load [copy]
154+
// CHECK-NOT: load [copy]
155+
// CHECK: } // end sil function 'remove_loadcopy_with_only_destroy_users'
156+
sil [ossa] @remove_loadcopy_with_only_destroy_users : $@convention(thin) (@in_guaranteed Builtin.NativeObject) -> () {
157+
bb0(%0 : $*Builtin.NativeObject):
158+
%1 = load [copy] %0 : $*Builtin.NativeObject
159+
destroy_value %1 : $Builtin.NativeObject
160+
%2 = load [copy] %0 : $*Builtin.NativeObject
161+
%f = function_ref @nativeobject_guaranteed_use : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
162+
apply %f(%2) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
163+
destroy_value %2 : $Builtin.NativeObject
164+
%9999 = tuple()
165+
return %9999 : $()
166+
}
167+
168+
// We should have a single load_borrow here.
169+
//
170+
// CHECK-LABEL: sil [ossa] @remove_loadborrow_with_only_destroy_users : $@convention(thin) (@in_guaranteed Builtin.NativeObject) -> () {
171+
// CHECK: load_borrow
172+
// CHECK-NOT: load_borrow
173+
// CHECK: } // end sil function 'remove_loadborrow_with_only_destroy_users'
174+
sil [ossa] @remove_loadborrow_with_only_destroy_users : $@convention(thin) (@in_guaranteed Builtin.NativeObject) -> () {
175+
bb0(%0 : $*Builtin.NativeObject):
176+
%1 = load_borrow %0 : $*Builtin.NativeObject
177+
end_borrow %1 : $Builtin.NativeObject
178+
%2 = load_borrow %0 : $*Builtin.NativeObject
179+
%f = function_ref @nativeobject_guaranteed_use : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
180+
apply %f(%2) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
181+
end_borrow %2 : $Builtin.NativeObject
182+
%9999 = tuple()
183+
return %9999 : $()
184+
}

0 commit comments

Comments
 (0)