Skip to content

Commit ecb5d65

Browse files
committed
Fix lifetime of intermediate phis created by RLE
We were adjusting the lifetime of the final phi created by the SSAUpdater. The intermediate phi's lifetime needs to be adjusted as well.
1 parent f5d47c3 commit ecb5d65

File tree

4 files changed

+85
-0
lines changed

4 files changed

+85
-0
lines changed

include/swift/SILOptimizer/Utils/InstOptUtils.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -703,6 +703,10 @@ makeCopiedValueAvailable(SILValue value, SILBasicBlock *inBlock);
703703
SILValue
704704
makeNewValueAvailable(SILValue value, SILBasicBlock *inBlock);
705705

706+
/// Given an ssa value \p value, create destroy_values at leaking blocks
707+
void endLifetimeAtLeakingBlocks(SILValue value,
708+
ArrayRef<SILBasicBlock *> userBBs);
709+
706710
/// Given a forwarding instruction, eliminate it if all of its users are debug
707711
/// instructions and ownership uses.
708712
bool tryEliminateOnlyOwnershipUsedForwardingInst(

lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,11 +1342,26 @@ SILValue RLEContext::computePredecessorLocationValue(SILBasicBlock *BB,
13421342
.getObjectType(),
13431343
Values[0].second.getOwnershipKind());
13441344

1345+
SmallVector<SILPhiArgument *, 8> insertedPhis;
1346+
Updater.setInsertedPhis(&insertedPhis);
1347+
13451348
for (auto V : Values) {
13461349
Updater.addAvailableValue(V.first, V.second);
13471350
}
13481351

13491352
auto Val = Updater.getValueInMiddleOfBlock(BB);
1353+
1354+
for (auto *phi : insertedPhis) {
1355+
if (phi == Val) {
1356+
continue;
1357+
}
1358+
// Fix lifetime of intermediate phis
1359+
SmallVector<SILBasicBlock *, 4> userBBs;
1360+
for (auto use : phi->getUses()) {
1361+
userBBs.push_back(use->getParentBlock());
1362+
}
1363+
endLifetimeAtLeakingBlocks(phi, userBBs);
1364+
}
13501365
return makeNewValueAvailable(Val, BB);
13511366
}
13521367

lib/SILOptimizer/Utils/InstOptUtils.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2060,3 +2060,22 @@ bool swift::tryEliminateOnlyOwnershipUsedForwardingInst(
20602060
callbacks.deleteInst(forwardingInst);
20612061
return true;
20622062
}
2063+
2064+
void swift::endLifetimeAtLeakingBlocks(SILValue value,
2065+
ArrayRef<SILBasicBlock *> uses) {
2066+
if (!value->getFunction()->hasOwnership())
2067+
return;
2068+
2069+
if (value.getOwnershipKind() != OwnershipKind::Owned)
2070+
return;
2071+
2072+
findJointPostDominatingSet(
2073+
value->getParentBlock(), uses, [&](SILBasicBlock *loopBlock) {},
2074+
[&](SILBasicBlock *postDomBlock) {
2075+
// Insert a destroy_value in the leaking block
2076+
auto front = postDomBlock->begin();
2077+
SILBuilderWithScope newBuilder(front);
2078+
newBuilder.createDestroyValue(
2079+
RegularLocation::getAutoGeneratedLocation(), value);
2080+
});
2081+
}

test/SILOptimizer/redundant_load_elim_ossa_complex.sil

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,3 +771,50 @@ bb7:
771771
return %val1 : $Klass
772772
}
773773

774+
// CHECK-LABEL: @rle_fixlifetimeof_intermediate_phis :
775+
// CHECK: load
776+
// CHECK-NOT: load
777+
// CHECK-LABEL: end sil function 'rle_fixlifetimeof_intermediate_phis'
778+
sil [ossa] @rle_fixlifetimeof_intermediate_phis : $@convention(thin) (@in NonTrivialStruct, @owned Klass, @owned Klass) -> @owned Klass {
779+
bb0(%0 : $*NonTrivialStruct, %1 : @owned $Klass, %2 : @owned $Klass):
780+
%ele = struct_element_addr %0 : $*NonTrivialStruct, #NonTrivialStruct.val
781+
%val1 = load [copy] %ele : $*Klass
782+
cond_br undef, bb1, bb2
783+
784+
bb1:
785+
store %2 to [assign] %ele : $*Klass
786+
br bb3
787+
788+
bb2:
789+
destroy_value %2 : $Klass
790+
br bb3
791+
792+
bb3:
793+
cond_br undef, bb4, bb8
794+
795+
bb4:
796+
cond_br undef, bb5, bb6
797+
798+
bb5:
799+
%val2 = load [copy] %ele : $*Klass
800+
destroy_value %val2 : $Klass
801+
store %1 to [assign] %ele : $*Klass
802+
br bb7
803+
804+
bb6:
805+
destroy_value %1 : $Klass
806+
br bb7
807+
808+
bb7:
809+
br bb9
810+
811+
bb8:
812+
destroy_value %1 : $Klass
813+
br bb9
814+
815+
bb9:
816+
%val3 = load [take] %ele : $*Klass
817+
destroy_value %val3 : $Klass
818+
return %val1 : $Klass
819+
}
820+

0 commit comments

Comments
 (0)