Skip to content

Commit 9a4a21d

Browse files
authored
Merge pull request #59541 from meg-gupta/mem2regstoreborrowpr
Handle store_borrow in SILMem2Reg and some other fixes
2 parents 699eddf + ed7788f commit 9a4a21d

File tree

7 files changed

+838
-262
lines changed

7 files changed

+838
-262
lines changed

include/swift/SILOptimizer/Utils/CFGOptUtils.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,14 @@ TermInst *changeEdgeValue(TermInst *branch, SILBasicBlock *dest, size_t idx,
6565
/// specified index. Asserts internally that the argument along the edge does
6666
/// not have uses.
6767
TermInst *deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
68-
size_t argIndex);
68+
size_t argIndex, bool cleanupDeadPhiOp = true);
6969

7070
/// Erase the \p argIndex phi argument from \p block. Asserts that the argument
7171
/// is a /real/ phi argument. Removes all incoming values for the argument from
7272
/// predecessor terminators. Asserts internally that it only ever is given
7373
/// "true" phi argument.
74-
void erasePhiArgument(SILBasicBlock *block, unsigned argIndex);
74+
void erasePhiArgument(SILBasicBlock *block, unsigned argIndex,
75+
bool cleanupDeadPhiOp = true);
7576

7677
/// Replace a branch target.
7778
///

lib/SIL/Verifier/LoadBorrowImmutabilityChecker.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ bool GatherWritesVisitor::visitUse(Operand *op, AccessUseType useTy) {
9494
case SILInstructionKind::SelectEnumAddrInst:
9595
case SILInstructionKind::SwitchEnumAddrInst:
9696
case SILInstructionKind::DeallocStackInst:
97+
case SILInstructionKind::DeallocStackRefInst:
9798
case SILInstructionKind::DeallocBoxInst:
9899
case SILInstructionKind::WitnessMethodInst:
99100
case SILInstructionKind::ExistentialMetatypeInst:

lib/SILOptimizer/Transforms/SILMem2Reg.cpp

Lines changed: 352 additions & 211 deletions
Large diffs are not rendered by default.

lib/SILOptimizer/Utils/CFGOptUtils.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ deleteTriviallyDeadOperandsOfDeadArgument(MutableArrayRef<Operand> termOperands,
8888
// Our implementation assumes that our caller is attempting to remove a dead
8989
// SILPhiArgument from a SILBasicBlock and has already RAUWed the argument.
9090
TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
91-
size_t argIndex) {
91+
size_t argIndex, bool cleanupDeadPhiOps) {
9292
if (auto *cbi = dyn_cast<CondBranchInst>(branch)) {
9393
SmallVector<SILValue, 8> trueArgs;
9494
SmallVector<SILValue, 8> falseArgs;
@@ -97,14 +97,18 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
9797
llvm::copy(cbi->getFalseArgs(), std::back_inserter(falseArgs));
9898

9999
if (destBlock == cbi->getTrueBB()) {
100-
deleteTriviallyDeadOperandsOfDeadArgument(cbi->getTrueOperands(),
101-
argIndex);
100+
if (cleanupDeadPhiOps) {
101+
deleteTriviallyDeadOperandsOfDeadArgument(cbi->getTrueOperands(),
102+
argIndex);
103+
}
102104
trueArgs.erase(trueArgs.begin() + argIndex);
103105
}
104106

105107
if (destBlock == cbi->getFalseBB()) {
106-
deleteTriviallyDeadOperandsOfDeadArgument(cbi->getFalseOperands(),
107-
argIndex);
108+
if (cleanupDeadPhiOps) {
109+
deleteTriviallyDeadOperandsOfDeadArgument(cbi->getFalseOperands(),
110+
argIndex);
111+
}
108112
falseArgs.erase(falseArgs.begin() + argIndex);
109113
}
110114

@@ -120,8 +124,9 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
120124
if (auto *bi = dyn_cast<BranchInst>(branch)) {
121125
SmallVector<SILValue, 8> args;
122126
llvm::copy(bi->getArgs(), std::back_inserter(args));
123-
124-
deleteTriviallyDeadOperandsOfDeadArgument(bi->getAllOperands(), argIndex);
127+
if (cleanupDeadPhiOps) {
128+
deleteTriviallyDeadOperandsOfDeadArgument(bi->getAllOperands(), argIndex);
129+
}
125130
args.erase(args.begin() + argIndex);
126131
auto *result = SILBuilderWithScope(bi).createBranch(bi->getLoc(),
127132
bi->getDestBB(), args);
@@ -132,7 +137,8 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
132137
llvm_unreachable("unsupported terminator");
133138
}
134139

135-
void swift::erasePhiArgument(SILBasicBlock *block, unsigned argIndex) {
140+
void swift::erasePhiArgument(SILBasicBlock *block, unsigned argIndex,
141+
bool cleanupDeadPhiOps) {
136142
assert(block->getArgument(argIndex)->isPhi()
137143
&& "Only should be used on phi arguments");
138144
block->eraseArgument(argIndex);
@@ -149,7 +155,7 @@ void swift::erasePhiArgument(SILBasicBlock *block, unsigned argIndex) {
149155
predBlocks.insert(pred);
150156

151157
for (auto *pred : predBlocks)
152-
deleteEdgeValue(pred->getTerminator(), block, argIndex);
158+
deleteEdgeValue(pred->getTerminator(), block, argIndex, cleanupDeadPhiOps);
153159
}
154160

155161
/// Changes the edge value between a branch and destination basic block

test/SILOptimizer/OSLogFullOptTest.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,15 @@ func testNSObjectInterpolation(nsArray: NSArray) {
131131
// TODO: check why the ARC optimizer cannot eliminate the many retain/release pairs here.
132132
// CHECK: entry:
133133
// CHECK-NEXT: bitcast %TSo7NSArrayC* %0 to i8*
134+
// CHECK-NEXT: [[COPY:%.+]] = tail call i8* @llvm.objc.retain
134135
// CHECK-NEXT: [[NSARRAY_ARG:%.+]] = tail call i8* @llvm.objc.retain
135136
// CHECK: tail call swiftcc i1 @"${{.*}}isLoggingEnabled{{.*}}"()
136137
// CHECK-NEXT: br i1 {{%.*}}, label %[[ENABLED:[0-9]+]], label %[[NOT_ENABLED:[0-9]+]]
137138

138139
// CHECK: [[NOT_ENABLED]]:
139140
// CHECK-NEXT: tail call void @swift_release
140141
// CHECK-NEXT: tail call void @llvm.objc.release
142+
// CHECK-NEXT: tail call void @llvm.objc.release
141143
// CHECK: br label %[[EXIT:[0-9]+]]
142144

143145
// CHECK: [[ENABLED]]:
@@ -167,6 +169,7 @@ func testNSObjectInterpolation(nsArray: NSArray) {
167169
// CHECK-NEXT: [[BITCASTED_SRC2:%.+]] = bitcast i8* {{.*}} to %TSo7NSArrayC*
168170
// CHECK-64-NEXT: store %TSo7NSArrayC* [[BITCASTED_SRC2]], %TSo7NSArrayC** [[BITCASTED_DEST2]], align 8
169171
// CHECK-32-NEXT: store %TSo7NSArrayC* [[BITCASTED_SRC2]], %TSo7NSArrayC** [[BITCASTED_DEST2]], align 4
172+
// CHECK-NEXT: tail call void @llvm.objc.release(i8* [[NSARRAY_ARG]])
170173
// CHECK-64: tail call swiftcc void @"${{.*}}_os_log_impl_test{{.*}}"({{.*}}, {{.*}}, {{.*}}, {{.*}}, i8* getelementptr inbounds ([20 x i8], [20 x i8]* @{{.*}}, i64 0, i64 0), i8* {{(nonnull )?}}[[BUFFER]], i32 12)
171174
// CHECK-32: tail call swiftcc void @"${{.*}}_os_log_impl_test{{.*}}"({{.*}}, {{.*}}, {{.*}}, {{.*}}, i8* getelementptr inbounds ([20 x i8], [20 x i8]* @{{.*}}, i32 0, i32 0), i8* {{(nonnull )?}}[[BUFFER]], i32 8)
172175
// CHECK: [[BITCASTED_OBJ_STORAGE:%.+]] = bitcast i8* [[OBJ_STORAGE]] to %swift.opaque*

test/SILOptimizer/mem2reg_borrows.sil

Lines changed: 10 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -79,23 +79,6 @@ bb0(%0 : @owned $Klass):
7979
return %6 : $()
8080
}
8181

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-
9982
// load_borrow of projections are not optimized
10083
// CHECK-LABEL: sil [ossa] @test_with_structs_and_borrows1 :
10184
// CHECK: alloc_stack
@@ -114,10 +97,10 @@ bb0(%0 : @guaranteed $WrapperStruct):
11497
%r = tuple ()
11598
return %r : $()
11699
}
117-
// CHECK-LABEL: sil [ossa] @store_only_allocas :
118-
// CHECK: alloc_stack
119-
// CHECK-LABEL: } // end sil function 'store_only_allocas'
120-
sil [ossa] @store_only_allocas : $@convention(thin) (@guaranteed Klass) -> () {
100+
// CHECK-LABEL: sil [ossa] @storeborrow_only_allocas :
101+
// CHECK-NOT: alloc_stack
102+
// CHECK-LABEL: } // end sil function 'storeborrow_only_allocas'
103+
sil [ossa] @storeborrow_only_allocas : $@convention(thin) (@guaranteed Klass) -> () {
121104
bb0(%0 : @guaranteed $Klass):
122105
%1 = alloc_stack $Klass
123106
%2 = store_borrow %0 to %1 : $*Klass
@@ -127,21 +110,8 @@ bb0(%0 : @guaranteed $Klass):
127110
return %6 : $()
128111
}
129112

130-
// CHECK-LABEL: sil [ossa] @store_only_lexicalallocas :
131-
// CHECK: alloc_stack
132-
// CHECK-LABEL: } // end sil function 'store_only_lexicalallocas'
133-
sil [ossa] @store_only_lexicalallocas : $@convention(thin) (@guaranteed Klass) -> () {
134-
bb0(%0 : @guaranteed $Klass):
135-
%1 = alloc_stack [lexical] $Klass
136-
%2 = store_borrow %0 to %1 : $*Klass
137-
end_borrow %2 : $*Klass
138-
dealloc_stack %1 : $*Klass
139-
%6 = tuple ()
140-
return %6 : $()
141-
}
142-
143113
// CHECK-LABEL: sil [ossa] @test1 :
144-
// CHECK: alloc_stack
114+
// CHECK-NOT: alloc_stack
145115
// CHECK-LABEL: } // end sil function 'test1'
146116
sil [ossa] @test1 : $@convention(thin) (@guaranteed Klass) -> () {
147117
bb0(%0 : @guaranteed $Klass):
@@ -158,7 +128,7 @@ bb0(%0 : @guaranteed $Klass):
158128
}
159129

160130
// CHECK-LABEL: sil [ossa] @test1_lexical :
161-
// CHECK: alloc_stack
131+
// CHECK-NOT: alloc_stack
162132
// CHECK-LABEL: } // end sil function 'test1_lexical'
163133
sil [ossa] @test1_lexical : $@convention(thin) (@guaranteed Klass) -> () {
164134
bb0(%0 : @guaranteed $Klass):
@@ -175,7 +145,7 @@ bb0(%0 : @guaranteed $Klass):
175145
}
176146

177147
// CHECK-LABEL: sil [ossa] @test2 :
178-
// CHECK: alloc_stack
148+
// CHECK-NOT: alloc_stack
179149
// CHECK-LABEL: } // end sil function 'test2'
180150
sil [ossa] @test2 : $@convention(thin) (@guaranteed Klass) -> () {
181151
bb0(%0 : @guaranteed $Klass):
@@ -196,7 +166,7 @@ bb0(%0 : @guaranteed $Klass):
196166
}
197167

198168
// CHECK-LABEL: sil [ossa] @test3 :
199-
// CHECK: alloc_stack
169+
// CHECK-NOT: alloc_stack
200170
// CHECK-LABEL: } // end sil function 'test3'
201171
sil [ossa] @test3 : $@convention(thin) (@guaranteed Klass, @guaranteed Klass) -> () {
202172
bb0(%0 : @guaranteed $Klass, %1 : @guaranteed $Klass):
@@ -218,7 +188,7 @@ bb0(%0 : @guaranteed $Klass, %1 : @guaranteed $Klass):
218188
}
219189

220190
// CHECK-LABEL: sil [ossa] @test4 :
221-
// CHECK: alloc_stack
191+
// CHECK-NOT: alloc_stack
222192
// CHECK-LABEL: } // end sil function 'test4'
223193
sil [ossa] @test4 : $@convention(thin) (@guaranteed Klass) -> () {
224194
bb0(%0 : @guaranteed $Klass):
@@ -238,7 +208,7 @@ bb0(%0 : @guaranteed $Klass):
238208
}
239209

240210
// CHECK-LABEL: sil [ossa] @test6 :
241-
// CHECK: alloc_stack
211+
// CHECK-NOT: alloc_stack
242212
// CHECK-LABEL: } // end sil function 'test6'
243213
sil [ossa] @test6 : $@convention(thin) (@owned Klass) -> () {
244214
bb0(%0 : @owned $Klass):

0 commit comments

Comments
 (0)