Skip to content

Commit 5b3c687

Browse files
committed
Fix an edge case in OSSA RLE for loops
In OSSA RLE for loops, in certain cases SSAUpdater will not create a new SILPhiArgument to be used as the forwarding value. Based on dominator info it may return the newly copied available value as the forwarding value. This newly copied available value in the dominating predecessor will have destroy values at leaking blocks. Rename makeNewValueAvailable to makeValueAvailable and handle users so that only additional required destroy_values are inserted.
1 parent 923c69d commit 5b3c687

File tree

6 files changed

+185
-16
lines changed

6 files changed

+185
-16
lines changed

include/swift/SILOptimizer/Utils/InstOptUtils.h

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -675,15 +675,10 @@ SILBasicBlock::iterator replaceSingleUse(Operand *use, SILValue newValue,
675675
SILValue
676676
makeCopiedValueAvailable(SILValue value, SILBasicBlock *inBlock);
677677

678-
/// Given a newly created @owned value \p value without any uses, this utility
678+
/// Given an existing @owned value \p value, this utility
679679
/// inserts control equivalent copy and destroy at leaking blocks to adjust
680680
/// ownership and make \p value available for use at \p inBlock.
681-
///
682-
/// inBlock must be the only point at which \p value will be consumed. If this
683-
/// consuming point is within a loop, this will create and return a copy of \p
684-
/// value inside \p inBlock.
685-
SILValue
686-
makeNewValueAvailable(SILValue value, SILBasicBlock *inBlock);
681+
SILValue makeValueAvailable(SILValue value, SILBasicBlock *inBlock);
687682

688683
/// Given an ssa value \p value, create destroy_values at leaking blocks
689684
///

lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1363,7 +1363,7 @@ SILValue RLEContext::computePredecessorLocationValue(SILBasicBlock *BB,
13631363
}
13641364
endLifetimeAtLeakingBlocks(phi, userBBs);
13651365
}
1366-
return makeNewValueAvailable(Val, BB);
1366+
return makeValueAvailable(Val, BB);
13671367
}
13681368

13691369
bool RLEContext::collectLocationValues(SILBasicBlock *BB, LSLocation &L,

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2672,7 +2672,7 @@ bool SimplifyCFG::simplifyTryApplyBlock(TryApplyInst *TAI) {
26722672
if (requiresOSSACleanup(newCallee)) {
26732673
newCallee = SILBuilderWithScope(newCallee->getNextInstruction())
26742674
.createCopyValue(calleeLoc, newCallee);
2675-
newCallee = makeNewValueAvailable(newCallee, TAI->getParent());
2675+
newCallee = makeValueAvailable(newCallee, TAI->getParent());
26762676
}
26772677

26782678
ApplyOptions Options = TAI->getApplyOptions();

lib/SILOptimizer/Utils/InstOptUtils.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2012,25 +2012,30 @@ SILValue swift::makeCopiedValueAvailable(SILValue value, SILBasicBlock *inBlock)
20122012
auto *copy = builder.createCopyValue(
20132013
RegularLocation::getAutoGeneratedLocation(), value);
20142014

2015-
return makeNewValueAvailable(copy, inBlock);
2015+
return makeValueAvailable(copy, inBlock);
20162016
}
20172017

2018-
SILValue swift::makeNewValueAvailable(SILValue value, SILBasicBlock *inBlock) {
2018+
SILValue swift::makeValueAvailable(SILValue value, SILBasicBlock *inBlock) {
20192019
if (!value->getFunction()->hasOwnership())
20202020
return value;
20212021

20222022
if (value.getOwnershipKind() == OwnershipKind::None)
20232023
return value;
20242024

2025-
assert(value->getUses().empty() &&
2026-
value.getOwnershipKind() == OwnershipKind::Owned);
2025+
assert(value.getOwnershipKind() == OwnershipKind::Owned);
2026+
2027+
SmallVector<SILBasicBlock *, 4> userBBs;
2028+
for (auto use : value->getUses()) {
2029+
userBBs.push_back(use->getParentBlock());
2030+
}
2031+
userBBs.push_back(inBlock);
20272032

20282033
// Use \p jointPostDomComputer to:
20292034
// 1. Create a control equivalent copy at \p inBlock if needed
20302035
// 2. Insert destroy_value at leaking blocks
20312036
SILValue controlEqCopy;
20322037
findJointPostDominatingSet(
2033-
value->getParentBlock(), inBlock,
2038+
value->getParentBlock(), userBBs,
20342039
[&](SILBasicBlock *loopBlock) {
20352040
assert(loopBlock == inBlock);
20362041
auto front = loopBlock->begin();

lib/SILOptimizer/Utils/LoadStoreOptUtils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ void LSValue::reduceInner(LSLocation &Base, SILModule *M,
119119
Builder, RegularLocation::getAutoGeneratedLocation(),
120120
Base.getType(M, context).getObjectType(), Vals);
121121

122-
auto AvailVal = makeNewValueAvailable(AI.get(), InsertPt->getParent());
122+
auto AvailVal = makeValueAvailable(AI.get(), InsertPt->getParent());
123123

124124
// This is the Value for the current base.
125125
ProjectionPath P(Base.getType(M, context));

test/SILOptimizer/redundant_load_elim_nontrivial_ossa.sil

Lines changed: 170 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,13 @@ final class NewHalfOpenRangeGenerator : NewRangeGenerator1 {
7878
sil_global @total_klass : $Klass
7979
sil_global @total_nontrivialstruct : $NonTrivialStruct
8080

81+
sil @use : $@convention(thin) (Builtin.Int32) -> ()
8182
sil @use_Klass : $@convention(thin) (@owned Klass) -> ()
8283
sil @use_nontrivialstruct : $@convention(thin) (@owned NonTrivialStruct) -> ()
8384
sil @use_a : $@convention(thin) (@owned A) -> ()
8485
sil @use_twofield : $@convention(thin) (@owned TwoField) -> ()
8586
sil @init_twofield : $@convention(thin) (@thin TwoField.Type) -> @owned TwoField
87+
sil @get_nontrivialstruct : $@convention(thin) () -> @owned NonTrivialStruct
8688

8789
// We have a bug in the old projection code which this test case exposes.
8890
// Make sure its handled properly in the new projection.
@@ -1051,7 +1053,6 @@ bb8:
10511053
unreachable
10521054
}
10531055

1054-
10551056
// CHECK-LABEL: @infinite_loop_and_unreachable :
10561057
// CHECK: [[V:%[0-9]+]] = load [copy]
10571058
// CHECK: [[C1:%[0-9]+]] = copy_value [[V]]
@@ -1078,3 +1079,171 @@ bb3:
10781079
unreachable
10791080
}
10801081

1082+
// CHECK-LABEL: @test_available_value1 :
1083+
// CHECK: load [trivial]
1084+
// CHECK-NOT: load [trivial]
1085+
// CHECK: } // end sil function 'test_available_value1'
1086+
sil [ossa] @test_available_value1 : $@convention(thin) (@in Builtin.Int32) -> () {
1087+
1088+
bb0(%0 : $*Builtin.Int32):
1089+
%1 = load [trivial] %0 : $*Builtin.Int32
1090+
1091+
%2 = function_ref @use : $@convention(thin) (Builtin.Int32) -> ()
1092+
%3 = apply %2(%1) : $@convention(thin) (Builtin.Int32) -> ()
1093+
cond_br undef, bb2, bb3
1094+
1095+
bb1:
1096+
cond_br undef, bb9, bb10
1097+
1098+
bb2:
1099+
br bb4
1100+
1101+
bb3:
1102+
br bb8
1103+
1104+
bb4:
1105+
br bb5
1106+
1107+
bb5:
1108+
1109+
%9 = function_ref @use : $@convention(thin) (Builtin.Int32) -> ()
1110+
%10 = apply %9(%1) : $@convention(thin) (Builtin.Int32) -> ()
1111+
cond_br undef, bb6, bb7
1112+
1113+
bb6:
1114+
br bb4
1115+
1116+
bb7:
1117+
br bb8
1118+
1119+
bb8:
1120+
br bb1
1121+
1122+
bb9:
1123+
br bb11
1124+
1125+
bb10:
1126+
br bb11
1127+
1128+
bb11:
1129+
%17 = tuple ()
1130+
return %17 : $()
1131+
}
1132+
1133+
sil [ossa] @test_available_value2 : $@convention(thin) (Builtin.Int32, Builtin.Int32) -> () {
1134+
bb0(%0 : $Builtin.Int32, %1 : $Builtin.Int32):
1135+
%2 = alloc_stack $Builtin.Int32
1136+
cond_br undef, bb1, bb2
1137+
1138+
bb1:
1139+
br bb3(%0 : $Builtin.Int32)
1140+
1141+
bb2:
1142+
br bb3(%1 : $Builtin.Int32)
1143+
1144+
1145+
bb3(%6 : $Builtin.Int32):
1146+
store %6 to [trivial] %2 : $*Builtin.Int32
1147+
cond_br undef, bb5, bb6
1148+
1149+
bb4:
1150+
cond_br undef, bb12, bb13
1151+
1152+
bb5:
1153+
br bb7
1154+
1155+
bb6:
1156+
br bb11
1157+
1158+
bb7:
1159+
br bb8
1160+
1161+
bb8:
1162+
1163+
%13 = function_ref @use : $@convention(thin) (Builtin.Int32) -> ()
1164+
%14 = apply %13(%6) : $@convention(thin) (Builtin.Int32) -> ()
1165+
cond_br undef, bb9, bb10
1166+
1167+
bb9:
1168+
br bb7
1169+
1170+
bb10:
1171+
br bb11
1172+
1173+
bb11:
1174+
br bb4
1175+
1176+
bb12:
1177+
br bb14
1178+
1179+
bb13:
1180+
br bb14
1181+
1182+
bb14:
1183+
dealloc_stack %2 : $*Builtin.Int32
1184+
%22 = tuple ()
1185+
return %22 : $()
1186+
}
1187+
1188+
// CHECK-LABEL: @test_available_value3 :
1189+
// CHECK-NOT: load
1190+
// CHECK: } // end sil function 'test_available_value3'
1191+
sil [ossa] @test_available_value3 : $@convention(method) (@owned NonTrivialStruct) -> () {
1192+
bb0(%0 : @owned $NonTrivialStruct):
1193+
%1 = alloc_stack $NonTrivialStruct
1194+
store %0 to [init] %1 : $*NonTrivialStruct
1195+
br bb1
1196+
1197+
bb1:
1198+
cond_br undef, bb2, bb3
1199+
1200+
bb2:
1201+
%5 = function_ref @get_nontrivialstruct : $@convention(thin) () -> @owned NonTrivialStruct
1202+
%6 = apply %5() : $@convention(thin) () -> @owned NonTrivialStruct
1203+
store %6 to [assign] %1 : $*NonTrivialStruct
1204+
cond_br undef, bb4, bb13
1205+
1206+
bb3:
1207+
unreachable
1208+
1209+
bb4:
1210+
cond_br undef, bb5, bb12
1211+
1212+
bb5:
1213+
br bb6
1214+
1215+
bb6:
1216+
br bb7
1217+
1218+
bb7:
1219+
cond_br undef, bb8, bb11
1220+
1221+
bb8:
1222+
cond_br undef, bb9, bb10
1223+
1224+
bb9:
1225+
%15 = load [take] %1 : $*NonTrivialStruct
1226+
destroy_value %15 : $NonTrivialStruct
1227+
dealloc_stack %1 : $*NonTrivialStruct
1228+
br bb14
1229+
1230+
bb10:
1231+
br bb6
1232+
1233+
bb11:
1234+
br bb1
1235+
1236+
bb12:
1237+
br bb1
1238+
1239+
bb13:
1240+
%22 = load [take] %1 : $*NonTrivialStruct
1241+
destroy_value %22 : $NonTrivialStruct
1242+
dealloc_stack %1 : $*NonTrivialStruct
1243+
br bb14
1244+
1245+
bb14:
1246+
%26 = tuple ()
1247+
return %26 : $()
1248+
}
1249+

0 commit comments

Comments
 (0)