Skip to content

Commit f7b443f

Browse files
committed
Enable mem2reg of enum types
Previously it was disabled because we have incomplete address lifetimes for such types, and transforming to value form in mem2reg would expose verification errors due to incompleteness. Enable this case here and fixup the lifetimes using the new lifetime completion utility.
1 parent 5a35479 commit f7b443f

File tree

4 files changed

+38
-22
lines changed

4 files changed

+38
-22
lines changed

lib/SILOptimizer/Transforms/SILMem2Reg.cpp

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "swift/SIL/BasicBlockDatastructures.h"
2727
#include "swift/SIL/Dominance.h"
2828
#include "swift/SIL/Projection.h"
29+
#include "swift/SIL/OSSALifetimeCompletion.h"
2930
#include "swift/SIL/SILBuilder.h"
3031
#include "swift/SIL/SILFunction.h"
3132
#include "swift/SIL/SILInstruction.h"
@@ -1784,13 +1785,42 @@ void StackAllocationPromoter::promoteAllocationToPhi() {
17841785
}
17851786

17861787
void StackAllocationPromoter::run() {
1788+
auto *function = asi->getFunction();
1789+
17871790
// Reduce the number of load/stores in the function to minimum.
17881791
// After this phase we are left with up to one load and store
17891792
// per block and the last store is recorded.
17901793
pruneAllocStackUsage();
17911794

17921795
// Replace AllocStacks with Phi-nodes.
17931796
promoteAllocationToPhi();
1797+
1798+
// Make sure that all of the allocations were promoted into registers.
1799+
assert(isWriteOnlyAllocation(asi) && "Non-write uses left behind");
1800+
1801+
SmallVector<SILValue> valuesToComplete;
1802+
1803+
// Enum types may have incomplete lifetimes in address form, when promoted to
1804+
// value form after mem2reg, they will end up with incomplete ossa lifetimes.
1805+
// Use the lifetime completion utility to complete such lifetimes.
1806+
// First, collect the stored values to complete.
1807+
if (asi->getType().isOrHasEnum()) {
1808+
for (auto it : initializationPoints) {
1809+
auto *si = it.second;
1810+
auto src = si->getOperand(0);
1811+
valuesToComplete.push_back(src);
1812+
}
1813+
}
1814+
1815+
// ... and erase the allocation.
1816+
deleter.forceDeleteWithUsers(asi);
1817+
1818+
// Now, complete lifetimes!
1819+
OSSALifetimeCompletion completion(function, domInfo);
1820+
1821+
for (auto it : valuesToComplete) {
1822+
completion.completeOSSALifetime(it);
1823+
}
17941824
}
17951825

17961826
//===----------------------------------------------------------------------===//
@@ -2090,14 +2120,6 @@ bool MemoryToRegisters::promoteSingleAllocation(AllocStackInst *alloc) {
20902120
<< *alloc);
20912121
deleter.forceDeleteWithUsers(alloc);
20922122
return true;
2093-
} else {
2094-
// For enums we require that all uses are in the same block.
2095-
// Otherwise there could be a switch_enum of an optional where the none-case
2096-
// does not have a destroy of the enum value.
2097-
// After transforming such an alloc_stack, the value would leak in the none-
2098-
// case block.
2099-
if (f.hasOwnership() && alloc->getType().isOrHasEnum())
2100-
return false;
21012123
}
21022124

21032125
LLVM_DEBUG(llvm::dbgs() << "*** Need to insert BB arguments for " << *alloc);
@@ -2108,11 +2130,6 @@ bool MemoryToRegisters::promoteSingleAllocation(AllocStackInst *alloc) {
21082130
StackAllocationPromoter(alloc, domInfo, domTreeLevels, ctx, deleter,
21092131
instructionsToDelete)
21102132
.run();
2111-
2112-
// Make sure that all of the allocations were promoted into registers.
2113-
assert(isWriteOnlyAllocation(alloc) && "Non-write uses left behind");
2114-
// ... and erase the allocation.
2115-
deleter.forceDeleteWithUsers(alloc);
21162133
return true;
21172134
}
21182135

test/SILOptimizer/mem2reg_lifetime_nontrivial.sil

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,8 +1061,7 @@ enum KlassOptional {
10611061
sil @return_optional_or_error : $@convention(thin) () -> (@owned KlassOptional, @error Error)
10621062

10631063
// CHECK-LABEL: sil [ossa] @test_deadphi4 :
1064-
// mem2reg cannot optimize an enum location which spans over multiple blocks.
1065-
// CHECK: alloc_stack
1064+
// CHECK-NOT: alloc_stack
10661065
// CHECK-LABEL: } // end sil function 'test_deadphi4'
10671066
sil [ossa] @test_deadphi4 : $@convention(thin) (@owned KlassOptional) -> () {
10681067
bb0(%0 : @owned $KlassOptional):

test/SILOptimizer/mem2reg_ossa.sil

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -489,10 +489,10 @@ bb0:
489489
return %16 : $((), ())
490490
}
491491

492-
// CHECK-LABEL: sil [ossa] @dont_optimize_optional_in_multiple_blocks :
493-
// CHECK: alloc_stack
494-
// CHECK: } // end sil function 'dont_optimize_optional_in_multiple_blocks'
495-
sil [ossa] @dont_optimize_optional_in_multiple_blocks : $@convention(method) (@guaranteed Optional<Klass>) -> () {
492+
// CHECK-LABEL: sil [ossa] @test_optional_in_multiple_blocks :
493+
// CHECK-NOT: alloc_stack
494+
// CHECK: } // end sil function 'test_optional_in_multiple_blocks'
495+
sil [ossa] @test_optional_in_multiple_blocks : $@convention(method) (@guaranteed Optional<Klass>) -> () {
496496
bb0(%0 : @guaranteed $Optional<Klass>):
497497
%1 = copy_value %0 : $Optional<Klass>
498498
%32 = alloc_stack $Optional<Klass>
@@ -504,8 +504,9 @@ bb5:
504504
br bb7
505505

506506
bb6(%50 : @guaranteed $Klass):
507-
%53 = load [take] %32 : $*Optional<Klass>
507+
%53 = load [copy] %32 : $*Optional<Klass>
508508
destroy_value %53 : $Optional<Klass>
509+
destroy_addr %32 : $*Optional<Klass>
509510
dealloc_stack %32 : $*Optional<Klass>
510511
br bb7
511512

test/SILOptimizer/mem2reg_ossa_nontrivial.sil

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,8 +1034,7 @@ enum KlassOptional {
10341034
sil @return_optional_or_error : $@convention(thin) () -> (@owned KlassOptional, @error Error)
10351035

10361036
// CHECK-LABEL: sil [ossa] @test_deadphi4 :
1037-
// mem2reg cannot optimize an enum location which spans over multiple blocks.
1038-
// CHECK: alloc_stack
1037+
// CHECK-NOT: alloc_stack
10391038
// CHECK-LABEL: } // end sil function 'test_deadphi4'
10401039
sil [ossa] @test_deadphi4 : $@convention(thin) (@owned KlassOptional) -> () {
10411040
bb0(%0 : @owned $KlassOptional):

0 commit comments

Comments
 (0)