Skip to content

Commit 344ef14

Browse files
meg-guptameg-gupta
committed
Fix load_borrow replacement in SILMem2Reg
1 parent 562e221 commit 344ef14

File tree

2 files changed

+72
-38
lines changed

2 files changed

+72
-38
lines changed

‎lib/SILOptimizer/Transforms/SILMem2Reg.cpp

Lines changed: 27 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,10 @@ struct LiveValues {
9393
/// For lexical AllocStackInsts, that is the copy made of the borrowed value.
9494
/// For those that are non-lexical, that is the value that was stored into the
9595
/// storage.
96-
SILValue replacement(AllocStackInst *asi) {
96+
SILValue replacement(AllocStackInst *asi, SILInstruction *toReplace) {
97+
if (isa<LoadBorrowInst>(toReplace)) {
98+
return shouldAddLexicalLifetime(asi) ? borrow : stored;
99+
}
97100
return shouldAddLexicalLifetime(asi) ? copy : stored;
98101
};
99102
};
@@ -601,22 +604,8 @@ StoreInst *StackAllocationPromoter::promoteAllocationInBlock(
601604

602605
if (isLoadFromStack(inst, asi)) {
603606
assert(!runningVals || runningVals->isStorageValid);
604-
if (auto *lbi = dyn_cast<LoadBorrowInst>(inst)) {
605-
if (runningVals) {
606-
if (shouldAddLexicalLifetime(asi)) {
607-
replaceLoad(lbi, runningVals->value.borrow, asi, ctx,
608-
deleter, instructionsToDelete);
609-
}
610-
else {
611-
replaceLoad(lbi, runningVals->value.replacement(asi), asi, ctx,
612-
deleter, instructionsToDelete);
613-
}
614-
++NumInstRemoved;
615-
}
616-
continue;
617-
}
618-
auto *li = cast<LoadInst>(inst);
619-
if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Take) {
607+
auto *li = dyn_cast<LoadInst>(inst);
608+
if (li && li->getOwnershipQualifier() == LoadOwnershipQualifier::Take) {
620609
if (shouldAddLexicalLifetime(asi)) {
621610
// End the lexical lifetime at a load [take]. The storage is no
622611
// longer keeping the value alive.
@@ -639,11 +628,11 @@ StoreInst *StackAllocationPromoter::promoteAllocationInBlock(
639628
if (runningVals) {
640629
// If we are loading from the AllocStackInst and we already know the
641630
// content of the Alloca then use it.
642-
LLVM_DEBUG(llvm::dbgs() << "*** Promoting load: " << *li);
643-
replaceLoad(inst, runningVals->value.replacement(asi), asi, ctx,
631+
LLVM_DEBUG(llvm::dbgs() << "*** Promoting load: " << *inst);
632+
replaceLoad(inst, runningVals->value.replacement(asi, inst), asi, ctx,
644633
deleter, instructionsToDelete);
645634
++NumInstRemoved;
646-
} else if (li->getOperand() == asi &&
635+
} else if (li && li->getOperand() == asi &&
647636
li->getOwnershipQualifier() != LoadOwnershipQualifier::Copy) {
648637
// If we don't know the content of the AllocStack then the loaded
649638
// value *is* the new value;
@@ -669,7 +658,7 @@ StoreInst *StackAllocationPromoter::promoteAllocationInBlock(
669658
if (runningVals) {
670659
assert(runningVals->isStorageValid);
671660
SILBuilderWithScope(si, ctx).createDestroyValue(
672-
si->getLoc(), runningVals->value.replacement(asi));
661+
si->getLoc(), runningVals->value.replacement(asi, si));
673662
} else {
674663
SILBuilderWithScope localBuilder(si, ctx);
675664
auto *newLoad = localBuilder.createLoad(si->getLoc(), asi,
@@ -736,17 +725,17 @@ StoreInst *StackAllocationPromoter::promoteAllocationInBlock(
736725
// promote this when we deal with hooking up phis.
737726
if (auto *dvi = DebugValueInst::hasAddrVal(inst)) {
738727
if (dvi->getOperand() == asi && runningVals)
739-
promoteDebugValueAddr(dvi, runningVals->value.replacement(asi), ctx,
740-
deleter);
728+
promoteDebugValueAddr(dvi, runningVals->value.replacement(asi, dvi),
729+
ctx, deleter);
741730
continue;
742731
}
743732

744733
// Replace destroys with a release of the value.
745734
if (auto *dai = dyn_cast<DestroyAddrInst>(inst)) {
746735
if (dai->getOperand() == asi) {
747736
if (runningVals) {
748-
replaceDestroy(dai, runningVals->value.replacement(asi), ctx, deleter,
749-
instructionsToDelete);
737+
replaceDestroy(dai, runningVals->value.replacement(asi, dai), ctx,
738+
deleter, instructionsToDelete);
750739
if (shouldAddLexicalLifetime(asi)) {
751740
endLexicalLifetimeBeforeInst(asi, /*beforeInstruction=*/dai, ctx,
752741
runningVals->value);
@@ -764,7 +753,7 @@ StoreInst *StackAllocationPromoter::promoteAllocationInBlock(
764753

765754
if (auto *dvi = dyn_cast<DestroyValueInst>(inst)) {
766755
if (runningVals &&
767-
dvi->getOperand() == runningVals->value.replacement(asi)) {
756+
dvi->getOperand() == runningVals->value.replacement(asi, dvi)) {
768757
// Reset LastStore.
769758
// So that we don't end up passing dead values as phi args in
770759
// StackAllocationPromoter::fixBranchesAndUses
@@ -928,7 +917,7 @@ void StackAllocationPromoter::fixPhiPredBlock(BlockSetVector &phiBlocks,
928917

929918
LiveValues def = getEffectiveLiveOutValues(phiBlocks, predBlock);
930919

931-
LLVM_DEBUG(llvm::dbgs() << "*** Found the definition: " << def.replacement(asi));
920+
LLVM_DEBUG(llvm::dbgs() << "*** Found the definition: " << def.stored);
932921

933922
llvm::SmallVector<SILValue> vals;
934923
vals.push_back(def.stored);
@@ -1009,10 +998,10 @@ void StackAllocationPromoter::fixBranchesAndUses(BlockSetVector &phiBlocks,
1009998
def = getEffectiveLiveInValues(phiBlocks, loadBlock);
1010999

10111000
LLVM_DEBUG(llvm::dbgs() << "*** Replacing " << *li << " with Def "
1012-
<< def.replacement(asi));
1001+
<< def.replacement(asi, li));
10131002

10141003
// Replace the load with the definition that we found.
1015-
replaceLoad(li, def.replacement(asi), asi, ctx, deleter,
1004+
replaceLoad(li, def.replacement(asi, li), asi, ctx, deleter,
10161005
instructionsToDelete);
10171006
removedUser = true;
10181007
++NumInstRemoved;
@@ -1030,15 +1019,15 @@ void StackAllocationPromoter::fixBranchesAndUses(BlockSetVector &phiBlocks,
10301019
// Replace debug_value w/ address-type value with
10311020
// a new debug_value w/ promoted value.
10321021
auto def = getEffectiveLiveInValues(phiBlocks, userBlock);
1033-
promoteDebugValueAddr(dvi, def.replacement(asi), ctx, deleter);
1022+
promoteDebugValueAddr(dvi, def.replacement(asi, dvi), ctx, deleter);
10341023
++NumInstRemoved;
10351024
continue;
10361025
}
10371026

10381027
// Replace destroys with a release of the value.
10391028
if (auto *dai = dyn_cast<DestroyAddrInst>(user)) {
10401029
auto def = getEffectiveLiveInValues(phiBlocks, userBlock);
1041-
replaceDestroy(dai, def.replacement(asi), ctx, deleter,
1030+
replaceDestroy(dai, def.replacement(asi, dai), ctx, deleter,
10421031
instructionsToDelete);
10431032
continue;
10441033
}
@@ -1638,8 +1627,8 @@ void MemoryToRegisters::removeSingleBlockAllocation(AllocStackInst *asi) {
16381627
}
16391628
runningVals->isStorageValid = false;
16401629
}
1641-
replaceLoad(inst, runningVals->value.replacement(asi), asi, ctx, deleter,
1642-
instructionsToDelete);
1630+
replaceLoad(inst, runningVals->value.replacement(asi, inst), asi, ctx,
1631+
deleter, instructionsToDelete);
16431632
++NumInstRemoved;
16441633
continue;
16451634
}
@@ -1651,7 +1640,7 @@ void MemoryToRegisters::removeSingleBlockAllocation(AllocStackInst *asi) {
16511640
if (si->getOwnershipQualifier() == StoreOwnershipQualifier::Assign) {
16521641
assert(runningVals && runningVals->isStorageValid);
16531642
SILBuilderWithScope(si, ctx).createDestroyValue(
1654-
si->getLoc(), runningVals->value.replacement(asi));
1643+
si->getLoc(), runningVals->value.replacement(asi, si));
16551644
}
16561645
auto oldRunningVals = runningVals;
16571646
runningVals = {LiveValues::toReplace(asi, /*replacement=*/si->getSrc()),
@@ -1674,8 +1663,8 @@ void MemoryToRegisters::removeSingleBlockAllocation(AllocStackInst *asi) {
16741663
if (auto *dvi = DebugValueInst::hasAddrVal(inst)) {
16751664
if (dvi->getOperand() == asi) {
16761665
if (runningVals) {
1677-
promoteDebugValueAddr(dvi, runningVals->value.replacement(asi), ctx,
1678-
deleter);
1666+
promoteDebugValueAddr(dvi, runningVals->value.replacement(asi, dvi),
1667+
ctx, deleter);
16791668
} else {
16801669
// Drop debug_value of uninitialized void values.
16811670
assert(asi->getElementType().isVoid() &&
@@ -1690,8 +1679,8 @@ void MemoryToRegisters::removeSingleBlockAllocation(AllocStackInst *asi) {
16901679
if (auto *dai = dyn_cast<DestroyAddrInst>(inst)) {
16911680
if (dai->getOperand() == asi) {
16921681
assert(runningVals && runningVals->isStorageValid);
1693-
replaceDestroy(dai, runningVals->value.replacement(asi), ctx, deleter,
1694-
instructionsToDelete);
1682+
replaceDestroy(dai, runningVals->value.replacement(asi, dai), ctx,
1683+
deleter, instructionsToDelete);
16951684
if (shouldAddLexicalLifetime(asi)) {
16961685
endLexicalLifetimeBeforeInst(asi, /*beforeInstruction=*/dai, ctx,
16971686
runningVals->value);

‎test/SILOptimizer/mem2reg_borrows.sil

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ struct WrapperStruct {
1010
sil [ossa] @use_inguaranteed : $@convention(thin) (@in_guaranteed Klass) -> ()
1111
sil [ossa] @use_guaranteed : $@convention(thin) (@guaranteed Klass) -> ()
1212
sil [ossa] @use_owned : $@convention(thin) (@owned Klass) -> ()
13+
sil [ossa] @get_owned : $@convention(thin) () -> @owned Klass
1314

1415
// CHECK-LABEL: sil [ossa] @test_no_storeborrow1 :
1516
// CHECK-NOT: alloc_stack
@@ -78,6 +79,23 @@ bb0(%0 : @owned $Klass):
7879
return %6 : $()
7980
}
8081

82+
// CHECK-LABEL: sil [ossa] @test_no_storeborrow4 :
83+
// CHECK-NOT: alloc_stack
84+
// CHECK-LABEL: } // end sil function 'test_no_storeborrow4'
85+
sil [ossa] @test_no_storeborrow4 : $@convention(thin) (@owned Klass) -> () {
86+
bb0(%0 : @owned $Klass):
87+
%1 = alloc_stack [lexical] $Klass
88+
store %0 to [init] %1 : $*Klass
89+
%2 = load_borrow %1 : $*Klass
90+
%3 = function_ref @use_guaranteed : $@convention(thin) (@guaranteed Klass) -> ()
91+
%4 = apply %3(%2) : $@convention(thin) (@guaranteed Klass) -> ()
92+
end_borrow %2 : $Klass
93+
destroy_addr %1 : $*Klass
94+
dealloc_stack %1 : $*Klass
95+
%6 = tuple ()
96+
return %6 : $()
97+
}
98+
8199
// load_borrow of projections are not optimized
82100
// CHECK-LABEL: sil [ossa] @test_with_structs_and_borrows1 :
83101
// CHECK: alloc_stack
@@ -371,3 +389,30 @@ bb3:
371389
return %8 : $()
372390
}
373391

392+
// CHECK-LABEL: sil [ossa] @test_control_flow6 :
393+
// CHECK-NOT: alloc_stack
394+
// CHECK-LABEL: } // end sil function 'test_control_flow6'
395+
sil [ossa] @test_control_flow6 : $@convention(thin) () -> () {
396+
bb0:
397+
%4 = alloc_stack [lexical] $Klass
398+
%f = function_ref @get_owned : $@convention(thin) () -> @owned Klass
399+
%5 = apply %f() : $@convention(thin) () -> @owned Klass
400+
store %5 to [init] %4 : $*Klass
401+
%7 = load_borrow %4 : $*Klass
402+
end_borrow %7 : $Klass
403+
cond_br undef, bb1, bb2
404+
405+
bb1:
406+
br bb3
407+
408+
bb2:
409+
%28 = load_borrow %4 : $*Klass
410+
end_borrow %28 : $Klass
411+
br bb3
412+
413+
bb3:
414+
destroy_addr %4 : $*Klass
415+
dealloc_stack %4 : $*Klass
416+
%r = tuple ()
417+
return %r : $()
418+
}

0 commit comments

Comments
 (0)