Skip to content

Commit 8347470

Browse files
committed
Enable SILMem2Reg for OSSA
SILMem2Reg has roughly 2 central algorithms, removal of alloc_stack with uses in a single block vs multiple blocks. While replacing loads and stores to the stack location in each of the 2 algorithms, new handling of qualifiers like [assign], [copy] and [take] which are new to OSSA are needed. Also Disable SILMem2Reg when we see this pattern: load [take] (struct_element_addr/tuple_element_addr %ASI) Convert SILMem2Reg tests into ossa And add new SILMem2Reg tests for non-trivial values. Thanks to zoecarver for additional tests.
1 parent eff6b66 commit 8347470

File tree

9 files changed

+1924
-24
lines changed

9 files changed

+1924
-24
lines changed

lib/SILOptimizer/Transforms/SILMem2Reg.cpp

Lines changed: 166 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -183,23 +183,43 @@ class MemoryToRegisters {
183183
/// Returns true if \p I is an address of a LoadInst, skipping struct and
184184
/// tuple address projections. Sets \p singleBlock to null if the load (or
185185
/// it's address is not in \p singleBlock.
186-
static bool isAddressForLoad(SILInstruction *I, SILBasicBlock *&singleBlock) {
187-
188-
if (isa<LoadInst>(I))
186+
/// This function looks for these patterns:
187+
/// 1. (load %ASI)
188+
/// 2. (load (struct_element_addr/tuple_element_addr/unchecked_addr_cast %ASI))
189+
static bool isAddressForLoad(SILInstruction *I, SILBasicBlock *&singleBlock,
190+
bool &hasGuaranteedOwnership) {
191+
192+
if (isa<LoadInst>(I)) {
193+
// SILMem2Reg is disabled when we find:
194+
// (load [take] (struct_element_addr/tuple_element_addr %ASI))
195+
// struct_element_addr and tuple_element_addr are lowered into
196+
// struct_extract and tuple_extract and these SIL instructions have a
197+
// guaranteed ownership. For replacing load's users, we need an owned value.
198+
// We will need a new copy and destroy of the running val placed after the
199+
// last use. This is not implemented currently.
200+
if (hasGuaranteedOwnership && cast<LoadInst>(I)->getOwnershipQualifier() ==
201+
LoadOwnershipQualifier::Take) {
202+
return false;
203+
}
189204
return true;
205+
}
190206

191207
if (!isa<UncheckedAddrCastInst>(I) && !isa<StructElementAddrInst>(I) &&
192208
!isa<TupleElementAddrInst>(I))
193209
return false;
194-
210+
211+
if (isa<StructElementAddrInst>(I) || isa<TupleElementAddrInst>(I)) {
212+
hasGuaranteedOwnership = true;
213+
}
214+
195215
// Recursively search for other (non-)loads in the instruction's uses.
196216
for (auto UI : cast<SingleValueInstruction>(I)->getUses()) {
197217
SILInstruction *II = UI->getUser();
198218
if (II->getParent() != singleBlock)
199219
singleBlock = nullptr;
200-
201-
if (!isAddressForLoad(II, singleBlock))
202-
return false;
220+
221+
if (!isAddressForLoad(II, singleBlock, hasGuaranteedOwnership))
222+
return false;
203223
}
204224
return true;
205225
}
@@ -233,7 +253,8 @@ static bool isCaptured(AllocStackInst *ASI, bool &inSingleBlock) {
233253
singleBlock = nullptr;
234254

235255
// Loads are okay.
236-
if (isAddressForLoad(II, singleBlock))
256+
bool hasGuaranteedOwnership = false;
257+
if (isAddressForLoad(II, singleBlock, hasGuaranteedOwnership))
237258
continue;
238259

239260
// We can store into an AllocStack (but not the pointer).
@@ -348,21 +369,54 @@ static void collectLoads(SILInstruction *I, SmallVectorImpl<LoadInst *> &Loads)
348369
static void replaceLoad(LoadInst *LI, SILValue val, AllocStackInst *ASI) {
349370
ProjectionPath projections(val->getType());
350371
SILValue op = LI->getOperand();
372+
SILBuilderWithScope builder(LI);
373+
351374
while (op != ASI) {
352375
assert(isa<UncheckedAddrCastInst>(op) || isa<StructElementAddrInst>(op) ||
353376
isa<TupleElementAddrInst>(op));
354377
auto *Inst = cast<SingleValueInstruction>(op);
355378
projections.push_back(Projection(Inst));
356379
op = Inst->getOperand(0);
357380
}
358-
SILBuilder builder(LI);
381+
382+
SmallVector<SILValue, 4> borrowedVals;
359383
for (auto iter = projections.rbegin(); iter != projections.rend(); ++iter) {
360384
const Projection &projection = *iter;
385+
assert(projection.getKind() == ProjectionKind::BitwiseCast ||
386+
projection.getKind() == ProjectionKind::Struct ||
387+
projection.getKind() == ProjectionKind::Tuple);
388+
389+
// struct_extract and tuple_extract expect guaranteed operand ownership
390+
// non-trivial RunningVal is owned. Insert borrow operation to convert
391+
if (projection.getKind() == ProjectionKind::Struct ||
392+
projection.getKind() == ProjectionKind::Tuple) {
393+
SILValue opVal = builder.emitBeginBorrowOperation(LI->getLoc(), val);
394+
if (opVal != val) {
395+
borrowedVals.push_back(opVal);
396+
val = opVal;
397+
}
398+
}
361399
val = projection.createObjectProjection(builder, LI->getLoc(), val).get();
362400
}
401+
363402
op = LI->getOperand();
364-
LI->replaceAllUsesWith(val);
403+
// Replace users of the loaded value with `val`
404+
// If we have a load [copy], replace the users with copy_value of `val`
405+
if (LI->getOwnershipQualifier() == LoadOwnershipQualifier::Copy) {
406+
LI->replaceAllUsesWith(builder.createCopyValue(LI->getLoc(), val));
407+
} else {
408+
assert(!ASI->getFunction()->hasOwnership() ||
409+
val.getOwnershipKind() != ValueOwnershipKind::Guaranteed);
410+
LI->replaceAllUsesWith(val);
411+
}
412+
413+
for (auto borrowedVal : borrowedVals) {
414+
builder.emitEndBorrowOperation(LI->getLoc(), borrowedVal);
415+
}
416+
417+
// Delete the load
365418
LI->eraseFromParent();
419+
366420
while (op != ASI && op->use_empty()) {
367421
assert(isa<UncheckedAddrCastInst>(op) || isa<StructElementAddrInst>(op) ||
368422
isa<TupleElementAddrInst>(op));
@@ -399,6 +453,7 @@ StoreInst *
399453
StackAllocationPromoter::promoteAllocationInBlock(SILBasicBlock *BB) {
400454
LLVM_DEBUG(llvm::dbgs() << "*** Promoting ASI in block: " << *ASI);
401455

456+
// RunningVal is the current value in the stack location.
402457
// We don't know the value of the alloca until we find the first store.
403458
SILValue RunningVal = SILValue();
404459
// Keep track of the last StoreInst that we found.
@@ -415,12 +470,16 @@ StackAllocationPromoter::promoteAllocationInBlock(SILBasicBlock *BB) {
415470
// If we are loading from the AllocStackInst and we already know the
416471
// content of the Alloca then use it.
417472
LLVM_DEBUG(llvm::dbgs() << "*** Promoting load: " << *Load);
418-
419473
replaceLoad(Load, RunningVal, ASI);
420474
++NumInstRemoved;
421-
} else if (Load->getOperand() == ASI) {
475+
} else if (Load->getOperand() == ASI &&
476+
Load->getOwnershipQualifier() !=
477+
LoadOwnershipQualifier::Copy) {
422478
// If we don't know the content of the AllocStack then the loaded
423479
// value *is* the new value;
480+
// Don't use result of load [copy] as a RunningVal, it necessitates
481+
// additional logic for cleanup of consuming instructions of the result.
482+
// StackAllocationPromoter::fixBranchesAndUses will later handle it.
424483
LLVM_DEBUG(llvm::dbgs() << "*** First load: " << *Load);
425484
RunningVal = Load;
426485
}
@@ -433,16 +492,51 @@ StackAllocationPromoter::promoteAllocationInBlock(SILBasicBlock *BB) {
433492
if (SI->getDest() != ASI)
434493
continue;
435494

436-
// The stored value is the new running value.
437-
RunningVal = SI->getSrc();
495+
// Special handling of entry block
496+
// If we have a store [assign] in the first block, OSSA guarantees we can
497+
// find the previous value stored in the stack location in RunningVal.
498+
// Create destroy_value of the RunningVal.
499+
// For all other blocks we may not know the previous value stored in the
500+
// stack location. So we will create destroy_value in
501+
// StackAllocationPromoter::fixBranchesAndUses, by getting the live-in
502+
// value to the block.
503+
if (BB->isEntry()) {
504+
if (SI->getOwnershipQualifier() == StoreOwnershipQualifier::Assign) {
505+
assert(RunningVal);
506+
SILBuilderWithScope(SI).createDestroyValue(SI->getLoc(), RunningVal);
507+
}
508+
}
438509

439510
// If we met a store before this one, delete it.
511+
// If the LastStore was a store with [assign], delete it only if we know
512+
// the RunningValue to destroy. If not, it will be deleted in
513+
// StackAllocationPromoter::fixBranchesAndUses.
440514
if (LastStore) {
441-
++NumInstRemoved;
442-
LLVM_DEBUG(llvm::dbgs() << "*** Removing redundant store: "
443-
<< *LastStore);
444-
LastStore->eraseFromParent();
515+
if (LastStore->getOwnershipQualifier() ==
516+
StoreOwnershipQualifier::Assign) {
517+
if (RunningVal) {
518+
// For entry block, we would have already created the destroy_value,
519+
// skip it.
520+
if (!BB->isEntry()) {
521+
SILBuilderWithScope(LastStore).createDestroyValue(
522+
LastStore->getLoc(), RunningVal);
523+
}
524+
LLVM_DEBUG(llvm::dbgs()
525+
<< "*** Removing redundant store: " << *LastStore);
526+
++NumInstRemoved;
527+
LastStore->eraseFromParent();
528+
}
529+
} else {
530+
LLVM_DEBUG(llvm::dbgs()
531+
<< "*** Removing redundant store: " << *LastStore);
532+
++NumInstRemoved;
533+
LastStore->eraseFromParent();
534+
}
445535
}
536+
537+
// The stored value is the new running value.
538+
RunningVal = SI->getSrc();
539+
// The current store is now the LastStore
446540
LastStore = SI;
447541
continue;
448542
}
@@ -466,6 +560,15 @@ StackAllocationPromoter::promoteAllocationInBlock(SILBasicBlock *BB) {
466560
continue;
467561
}
468562

563+
if (auto *DVI = dyn_cast<DestroyValueInst>(Inst)) {
564+
if (DVI->getOperand() == RunningVal) {
565+
// Reset LastStore.
566+
// So that we don't end up passing dead values as phi args in
567+
// StackAllocationPromoter::fixBranchesAndUses
568+
LastStore = nullptr;
569+
}
570+
}
571+
469572
// Stop on deallocation.
470573
if (auto *DSI = dyn_cast<DeallocStackInst>(Inst)) {
471574
if (DSI->getOperand() == ASI) {
@@ -516,6 +619,10 @@ void MemoryToRegisters::removeSingleBlockAllocation(AllocStackInst *ASI) {
516619
// value.
517620
if (auto *SI = dyn_cast<StoreInst>(Inst)) {
518621
if (SI->getDest() == ASI) {
622+
if (SI->getOwnershipQualifier() == StoreOwnershipQualifier::Assign) {
623+
assert(RunningVal);
624+
SILBuilderWithScope(SI).createDestroyValue(SI->getLoc(), RunningVal);
625+
}
519626
RunningVal = SI->getSrc();
520627
Inst->eraseFromParent();
521628
++NumInstRemoved;
@@ -647,6 +754,21 @@ void StackAllocationPromoter::fixPhiPredBlock(BlockSet &PhiBlocks,
647754
TI->eraseFromParent();
648755
}
649756

757+
static bool hasOnlyUndefIncomingValues(SILPhiArgument *phiArg) {
758+
SmallVector<SILValue, 8> incomingValues;
759+
phiArg->getIncomingPhiValues(incomingValues);
760+
for (auto predArg : incomingValues) {
761+
if (isa<SILUndef>(predArg))
762+
continue;
763+
if (isa<SILPhiArgument>(predArg) &&
764+
hasOnlyUndefIncomingValues(cast<SILPhiArgument>(predArg))) {
765+
continue;
766+
}
767+
return false;
768+
}
769+
return true;
770+
}
771+
650772
void StackAllocationPromoter::fixBranchesAndUses(BlockSet &PhiBlocks) {
651773
// First update uses of the value.
652774
SmallVector<LoadInst *, 4> collectedLoads;
@@ -683,6 +805,16 @@ void StackAllocationPromoter::fixBranchesAndUses(BlockSet &PhiBlocks) {
683805
// on.
684806
SILBasicBlock *BB = Inst->getParent();
685807

808+
if (!BB->isEntry()) {
809+
if (auto *SI = dyn_cast<StoreInst>(Inst)) {
810+
if (SI->getOwnershipQualifier() == StoreOwnershipQualifier::Assign) {
811+
SILValue Def = getLiveInValue(PhiBlocks, BB);
812+
SILBuilderWithScope(SI).createDestroyValue(SI->getLoc(), Def);
813+
continue;
814+
}
815+
}
816+
}
817+
686818
if (auto *DVAI = dyn_cast<DebugValueAddrInst>(Inst)) {
687819
// Replace DebugValueAddr with DebugValue.
688820
SILValue Def = getLiveInValue(PhiBlocks, BB);
@@ -714,6 +846,22 @@ void StackAllocationPromoter::fixBranchesAndUses(BlockSet &PhiBlocks) {
714846
fixPhiPredBlock(PhiBlocks, Block, PBB);
715847
}
716848
}
849+
850+
// If the owned phi arg we added did not have any uses, create end_lifetime to
851+
// end its lifetime. In asserts mode, make sure we have only undef incoming
852+
// values for such phi args.
853+
if (ASI->getFunction()->hasOwnership()) {
854+
for (auto Block : PhiBlocks) {
855+
auto *phiArg = cast<SILPhiArgument>(
856+
Block->getArgument(Block->getNumArguments() - 1));
857+
if (phiArg->getOwnershipKind() == ValueOwnershipKind::Owned &&
858+
phiArg->use_empty()) {
859+
assert(hasOnlyUndefIncomingValues(phiArg));
860+
SILBuilderWithScope(&Block->front())
861+
.createEndLifetime(Block->front().getLoc(), phiArg);
862+
}
863+
}
864+
}
717865
}
718866

719867
void StackAllocationPromoter::pruneAllocStackUsage() {
@@ -960,10 +1108,6 @@ class SILMem2Reg : public SILFunctionTransform {
9601108
void run() override {
9611109
SILFunction *F = getFunction();
9621110

963-
// FIXME: We should be able to support ownership.
964-
if (F->hasOwnership())
965-
return;
966-
9671111
LLVM_DEBUG(llvm::dbgs() << "** Mem2Reg on function: " << F->getName()
9681112
<< " **\n");
9691113

test/SILOptimizer/mem2reg.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,12 +449,12 @@ bb0:
449449
return %16 : $((), ())
450450
}
451451

452-
// CHECK-LABEL: sil @unchecked_ref_cast
452+
// CHECK-LABEL: sil @unchecked_bitwise_cast
453453
// CHECK-NOT: alloc_stack
454454
// CHECK: [[CAST:%.*]] = unchecked_bitwise_cast %0 : $Optional<Klass> to $Klass
455455
// CHECK: release_value [[CAST]]
456456
// CHECK: return
457-
sil @unchecked_ref_cast : $@convention(thin) (@owned Optional<Klass>) -> () {
457+
sil @unchecked_bitwise_cast : $@convention(thin) (@owned Optional<Klass>) -> () {
458458
bb0(%0 : $Optional<Klass>):
459459
%1 = alloc_stack $Optional<Klass>
460460
store %0 to %1 : $*Optional<Klass>
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all %s -mem2reg | %FileCheck %s
2+
3+
import Builtin
4+
import Swift
5+
6+
sil [ossa] @_Ts5printFT3valSi_T_ : $@convention(thin) (Int64) -> ()
7+
8+
// CHECK-LABEL: sil [ossa] @liveness0 :
9+
// CHECK-NOT: alloc_stack
10+
sil [ossa] @liveness0 : $@convention(thin) (Int64) -> () {
11+
bb0(%0 : $Int64):
12+
%1 = alloc_stack $Int64, var, name "x"
13+
store %0 to [trivial] %1 : $*Int64
14+
%3 = integer_literal $Builtin.Int64, 10
15+
%5 = struct_extract %0 : $Int64, #Int64._value
16+
%6 = builtin "cmp_eq_Int64"(%5 : $Builtin.Int64, %3 : $Builtin.Int64) : $Builtin.Int1
17+
cond_br %6, bb1, bb5
18+
19+
bb1:
20+
%8 = alloc_stack $Int64, var, name "y"
21+
%9 = integer_literal $Builtin.Int64, 20
22+
%10 = struct $Int64 (%9 : $Builtin.Int64)
23+
store %10 to [trivial] %8 : $*Int64
24+
%12 = integer_literal $Builtin.Int64, 3
25+
%14 = struct_extract %0 : $Int64, #Int64._value
26+
%15 = builtin "cmp_sgt_Int64"(%14 : $Builtin.Int64, %12 : $Builtin.Int64) : $Builtin.Int1
27+
cond_br %15, bb2, bb3
28+
29+
bb2:
30+
%17 = integer_literal $Builtin.Int64, 0
31+
%18 = struct $Int64 (%17 : $Builtin.Int64)
32+
store %18 to [trivial] %8 : $*Int64
33+
br bb4
34+
35+
bb3:
36+
%21 = integer_literal $Builtin.Int64, 2
37+
%22 = struct $Int64 (%21 : $Builtin.Int64)
38+
store %22 to [trivial] %8 : $*Int64
39+
br bb4
40+
41+
bb4:
42+
// function_ref
43+
%25 = function_ref @_Ts5printFT3valSi_T_ : $@convention(thin) (Int64) -> ()
44+
%26 = load [trivial] %8 : $*Int64
45+
%27 = integer_literal $Builtin.Int64, 2
46+
%28 = integer_literal $Builtin.Int1, -1
47+
%30 = struct_extract %26 : $Int64, #Int64._value
48+
%31 = builtin "sadd_with_overflow_Int64"(%30 : $Builtin.Int64, %27 : $Builtin.Int64, %28 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
49+
%32 = tuple_extract %31 : $(Builtin.Int64, Builtin.Int1), 0
50+
%33 = tuple_extract %31 : $(Builtin.Int64, Builtin.Int1), 1
51+
%34 = struct $Int64 (%32 : $Builtin.Int64)
52+
cond_fail %33 : $Builtin.Int1
53+
%36 = apply %25(%34) : $@convention(thin) (Int64) -> ()
54+
dealloc_stack %8 : $*Int64
55+
br bb5
56+
57+
// We don't need a PHI node here because the value is dead!
58+
// CHECK: bb5:
59+
bb5:
60+
dealloc_stack %1 : $*Int64
61+
%40 = tuple ()
62+
return %40 : $()
63+
}
64+
// CHECK-LABEL: } // end sil function 'liveness0'
65+

0 commit comments

Comments
 (0)