Skip to content

Commit fefd027

Browse files
gottesmmgottesmm
committed
[semantic-arc-opts] Convert @owned -> @guaranteed args of transforming terminators when fed by load [copy], copy_value.
This extends the (copy (guaranteed x)) -> (guaranteed x) optimization to handle transforming terminator arguments. This will begin to enable us to eliminate RC ops around optionals or casts. I still need to add support for eliminating copies that forward through br args and phis. <rdar://problem/56720436> <rdar://problem/56720519>
1 parent d8bc35b commit fefd027

File tree

3 files changed

+263
-20
lines changed

3 files changed

+263
-20
lines changed

‎lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,19 +113,20 @@ LiveRange::LiveRange(SILValue value)
113113
continue;
114114
}
115115

116-
// Otherwise, we have some form of consuming use that either forwards or
117-
// that we do not understand. See if we have a forwarding value that has a
118-
// single non-trivial operand that can accept a guaranteed value. If not, we
119-
// can not recursively process it, so be conservative and assume that we
120-
// /may consume/ the value, so the live range must not be eliminated.
116+
// Otherwise, see if we have a forwarding value that has a single
117+
// non-trivial operand that can accept a guaranteed value. If not, we can
118+
// not recursively process it, so be conservative and assume that we /may
119+
// consume/ the value, so the live range must not be eliminated.
121120
//
122121
// DISCUSSION: For now we do not support forwarding instructions with
123122
// multiple non-trivial arguments since we would need to optimize all of
124123
// the non-trivial arguments at the same time.
125124
//
126125
// NOTE: Today we do not support TermInsts for simplicity... we /could/
127126
// support it though if we need to.
128-
if (isa<TermInst>(user) || !isGuaranteedForwardingInst(user) ||
127+
auto *ti = dyn_cast<TermInst>(user);
128+
if ((ti && !ti->isTransformationTerminator()) ||
129+
!isGuaranteedForwardingInst(user) ||
129130
1 != count_if(user->getOperandValues(
130131
true /*ignore type dependent operands*/),
131132
[&](SILValue v) {
@@ -137,13 +138,39 @@ LiveRange::LiveRange(SILValue value)
137138
}
138139

139140
// Ok, this is a forwarding instruction whose ownership we can flip from
140-
// owned -> guaranteed. Visit its users recursively to see if the the
141-
// users force the live range to be alive.
141+
// owned -> guaranteed.
142142
generalForwardingInsts.push_back(user);
143-
for (SILValue v : user->getResults()) {
144-
if (v.getOwnershipKind() != ValueOwnershipKind::Owned)
143+
144+
// If we have a non-terminator, just visit its users recursively to see if
145+
// the the users force the live range to be alive.
146+
if (!ti) {
147+
for (SILValue v : user->getResults()) {
148+
if (v.getOwnershipKind() != ValueOwnershipKind::Owned)
149+
continue;
150+
llvm::copy(v->getUses(), std::back_inserter(worklist));
151+
}
152+
continue;
153+
}
154+
155+
// Otherwise, we know that we have no only a terminator, but a
156+
// transformation terminator, so we should add the users of its results to
157+
// the worklist.
158+
for (auto &succ : ti->getSuccessors()) {
159+
auto *succBlock = succ.getBB();
160+
161+
// If we do not have any arguments, then continue.
162+
if (succBlock->args_empty())
145163
continue;
146-
llvm::copy(v->getUses(), std::back_inserter(worklist));
164+
165+
for (auto *succArg : succBlock->getSILPhiArguments()) {
166+
// If we have an any value, just continue.
167+
if (succArg->getOwnershipKind() == ValueOwnershipKind::None)
168+
continue;
169+
170+
// Otherwise add all users of this BBArg to the worklist to visit
171+
// recursively.
172+
llvm::copy(succArg->getUses(), std::back_inserter(worklist));
173+
}
147174
}
148175
}
149176
}
@@ -519,8 +546,28 @@ static void convertForwardingInstsFromOwnedToGuaranteed(
519546
while (!guaranteedForwardingInsts.empty()) {
520547
auto *i = guaranteedForwardingInsts.back();
521548
guaranteedForwardingInsts = guaranteedForwardingInsts.drop_back();
522-
assert(i->hasResults());
523549

550+
// If this is a term inst, just convert all of its incoming values that are
551+
// owned to be guaranteed.
552+
if (auto *ti = dyn_cast<TermInst>(i)) {
553+
for (auto &succ : ti->getSuccessors()) {
554+
auto *succBlock = succ.getBB();
555+
556+
// If we do not have any arguments, then continue.
557+
if (succBlock->args_empty())
558+
continue;
559+
560+
for (auto *succArg : succBlock->getSILPhiArguments()) {
561+
// If we have an any value, just continue.
562+
if (succArg->getOwnershipKind() == ValueOwnershipKind::Owned) {
563+
succArg->setOwnershipKind(ValueOwnershipKind::Guaranteed);
564+
}
565+
}
566+
}
567+
continue;
568+
}
569+
570+
assert(i->hasResults());
524571
for (SILValue result : i->getResults()) {
525572
if (auto *svi = dyn_cast<OwnershipForwardingSingleValueInst>(result)) {
526573
if (svi->getOwnershipKind() == ValueOwnershipKind::Owned) {

‎test/SILOptimizer/semantic-arc-opts-canonical.sil

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -342,10 +342,8 @@ bb0(%0 : @guaranteed $Builtin.NativeObject, %1 : @guaranteed $Builtin.NativeObje
342342
return %9999 : $()
343343
}
344344

345-
// TODO: We should be able to optimize these switch_enum. Make sure that today
346-
// we do not do so though.
347345
// CHECK-LABEL: sil [ossa] @switch_enum_test_no_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
348-
// CHECK: copy_value
346+
// CHECK-NOT: copy_value
349347
// CHECK: } // end sil function 'switch_enum_test_no_default'
350348
sil [ossa] @switch_enum_test_no_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
351349
bb0(%0 : @guaranteed $FakeOptional<Builtin.NativeObject>):
@@ -365,7 +363,7 @@ bb3:
365363
}
366364

367365
// CHECK-LABEL: sil [ossa] @switch_enum_test_with_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
368-
// CHECK: copy_value
366+
// CHECK-NOT: copy_value
369367
// CHECK: } // end sil function 'switch_enum_test_with_default'
370368
sil [ossa] @switch_enum_test_with_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
371369
bb0(%0 : @guaranteed $FakeOptional<Builtin.NativeObject>):
@@ -434,3 +432,56 @@ bb3(%6 : $*FakeOptional<Klass>):
434432
%9999 = tuple()
435433
return %9999 : $()
436434
}
435+
436+
// CHECK-LABEL: sil [ossa] @switch_enum_test_loadcopy_no_default : $@convention(thin) (@owned FakeOptional<Builtin.NativeObject>) -> () {
437+
// CHECK-NOT: load [copy]
438+
// CHECK: load_borrow
439+
// CHECK-NOT: load [copy]
440+
// CHECK: } // end sil function 'switch_enum_test_loadcopy_no_default'
441+
sil [ossa] @switch_enum_test_loadcopy_no_default : $@convention(thin) (@owned FakeOptional<Builtin.NativeObject>) -> () {
442+
bb0(%0 : @owned $FakeOptional<Builtin.NativeObject>):
443+
%0a = alloc_stack $FakeOptional<Builtin.NativeObject>
444+
store %0 to [init] %0a : $*FakeOptional<Builtin.NativeObject>
445+
%1 = load [copy] %0a : $*FakeOptional<Builtin.NativeObject>
446+
switch_enum %1 : $FakeOptional<Builtin.NativeObject>, case #FakeOptional.some!enumelt.1: bb1, case #FakeOptional.none!enumelt: bb2
447+
448+
bb1(%2 : @owned $Builtin.NativeObject):
449+
destroy_value %2 : $Builtin.NativeObject
450+
br bb3
451+
452+
bb2:
453+
br bb3
454+
455+
bb3:
456+
destroy_addr %0a : $*FakeOptional<Builtin.NativeObject>
457+
dealloc_stack %0a : $*FakeOptional<Builtin.NativeObject>
458+
%9999 = tuple()
459+
return %9999 : $()
460+
}
461+
462+
// CHECK-LABEL: sil [ossa] @switch_enum_test_loadcopy_with_default : $@convention(thin) (@owned FakeOptional<Builtin.NativeObject>) -> () {
463+
// CHECK-NOT: load [copy]
464+
// CHECK: load_borrow
465+
// CHECK-NOT: load [copy]
466+
// CHECK: } // end sil function 'switch_enum_test_loadcopy_with_default'
467+
sil [ossa] @switch_enum_test_loadcopy_with_default : $@convention(thin) (@owned FakeOptional<Builtin.NativeObject>) -> () {
468+
bb0(%0 : @owned $FakeOptional<Builtin.NativeObject>):
469+
%0a = alloc_stack $FakeOptional<Builtin.NativeObject>
470+
store %0 to [init] %0a : $*FakeOptional<Builtin.NativeObject>
471+
%1 = load [copy] %0a : $*FakeOptional<Builtin.NativeObject>
472+
switch_enum %1 : $FakeOptional<Builtin.NativeObject>, case #FakeOptional.some!enumelt.1: bb1, default bb2
473+
474+
bb1(%2 : @owned $Builtin.NativeObject):
475+
destroy_value %2 : $Builtin.NativeObject
476+
br bb3
477+
478+
bb2(%3 : @owned $FakeOptional<Builtin.NativeObject>):
479+
destroy_value %3 : $FakeOptional<Builtin.NativeObject>
480+
br bb3
481+
482+
bb3:
483+
destroy_addr %0a : $*FakeOptional<Builtin.NativeObject>
484+
dealloc_stack %0a : $*FakeOptional<Builtin.NativeObject>
485+
%9999 = tuple()
486+
return %9999 : $()
487+
}

‎test/SILOptimizer/semantic-arc-opts.sil

Lines changed: 149 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -360,10 +360,8 @@ bb0(%0 : @guaranteed $Builtin.NativeObject, %1 : @guaranteed $Builtin.NativeObje
360360
return %9999 : $()
361361
}
362362

363-
// TODO: We should be able to optimize these switch_enum. Make sure that today
364-
// we do not do so though.
365363
// CHECK-LABEL: sil [ossa] @switch_enum_test_no_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
366-
// CHECK: copy_value
364+
// CHECK-NOT: copy_value
367365
// CHECK: } // end sil function 'switch_enum_test_no_default'
368366
sil [ossa] @switch_enum_test_no_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
369367
bb0(%0 : @guaranteed $FakeOptional<Builtin.NativeObject>):
@@ -383,7 +381,7 @@ bb3:
383381
}
384382

385383
// CHECK-LABEL: sil [ossa] @switch_enum_test_with_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
386-
// CHECK: copy_value
384+
// CHECK-NOT: copy_value
387385
// CHECK: } // end sil function 'switch_enum_test_with_default'
388386
sil [ossa] @switch_enum_test_with_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
389387
bb0(%0 : @guaranteed $FakeOptional<Builtin.NativeObject>):
@@ -1140,3 +1138,150 @@ bb0(%0 : $*NativeObjectPair, %1 : @owned $Builtin.NativeObject):
11401138
%9999 = tuple()
11411139
return %9999 : $()
11421140
}
1141+
1142+
// CHECK-LABEL: sil [ossa] @switch_enum_test_loadcopy_no_default : $@convention(thin) (@owned FakeOptional<Builtin.NativeObject>) -> () {
1143+
// CHECK-NOT: load [copy]
1144+
// CHECK: load_borrow
1145+
// CHECK-NOT: load [copy]
1146+
// CHECK: } // end sil function 'switch_enum_test_loadcopy_no_default'
1147+
sil [ossa] @switch_enum_test_loadcopy_no_default : $@convention(thin) (@owned FakeOptional<Builtin.NativeObject>) -> () {
1148+
bb0(%0 : @owned $FakeOptional<Builtin.NativeObject>):
1149+
%0a = alloc_stack $FakeOptional<Builtin.NativeObject>
1150+
store %0 to [init] %0a : $*FakeOptional<Builtin.NativeObject>
1151+
%1 = load [copy] %0a : $*FakeOptional<Builtin.NativeObject>
1152+
switch_enum %1 : $FakeOptional<Builtin.NativeObject>, case #FakeOptional.some!enumelt.1: bb1, case #FakeOptional.none!enumelt: bb2
1153+
1154+
bb1(%2 : @owned $Builtin.NativeObject):
1155+
destroy_value %2 : $Builtin.NativeObject
1156+
br bb3
1157+
1158+
bb2:
1159+
br bb3
1160+
1161+
bb3:
1162+
destroy_addr %0a : $*FakeOptional<Builtin.NativeObject>
1163+
dealloc_stack %0a : $*FakeOptional<Builtin.NativeObject>
1164+
%9999 = tuple()
1165+
return %9999 : $()
1166+
}
1167+
1168+
// CHECK-LABEL: sil [ossa] @switch_enum_test_loadcopy_with_default : $@convention(thin) (@owned FakeOptional<Builtin.NativeObject>) -> () {
1169+
// CHECK-NOT: load [copy]
1170+
// CHECK: load_borrow
1171+
// CHECK-NOT: load [copy]
1172+
// CHECK: } // end sil function 'switch_enum_test_loadcopy_with_default'
1173+
sil [ossa] @switch_enum_test_loadcopy_with_default : $@convention(thin) (@owned FakeOptional<Builtin.NativeObject>) -> () {
1174+
bb0(%0 : @owned $FakeOptional<Builtin.NativeObject>):
1175+
%0a = alloc_stack $FakeOptional<Builtin.NativeObject>
1176+
store %0 to [init] %0a : $*FakeOptional<Builtin.NativeObject>
1177+
%1 = load [copy] %0a : $*FakeOptional<Builtin.NativeObject>
1178+
switch_enum %1 : $FakeOptional<Builtin.NativeObject>, case #FakeOptional.some!enumelt.1: bb1, default bb2
1179+
1180+
bb1(%2 : @owned $Builtin.NativeObject):
1181+
destroy_value %2 : $Builtin.NativeObject
1182+
br bb3
1183+
1184+
bb2(%3 : @owned $FakeOptional<Builtin.NativeObject>):
1185+
destroy_value %3 : $FakeOptional<Builtin.NativeObject>
1186+
br bb3
1187+
1188+
bb3:
1189+
destroy_addr %0a : $*FakeOptional<Builtin.NativeObject>
1190+
dealloc_stack %0a : $*FakeOptional<Builtin.NativeObject>
1191+
%9999 = tuple()
1192+
return %9999 : $()
1193+
}
1194+
1195+
// CHECK-LABEL: sil [ossa] @switch_enum_test_copyvalue_no_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
1196+
// CHECK-NOT: copy_value
1197+
// CHECK: } // end sil function 'switch_enum_test_copyvalue_no_default'
1198+
sil [ossa] @switch_enum_test_copyvalue_no_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
1199+
bb0(%0 : @guaranteed $FakeOptional<Builtin.NativeObject>):
1200+
%1 = copy_value %0 : $FakeOptional<Builtin.NativeObject>
1201+
switch_enum %1 : $FakeOptional<Builtin.NativeObject>, case #FakeOptional.some!enumelt.1: bb1, case #FakeOptional.none!enumelt: bb2
1202+
1203+
bb1(%2 : @owned $Builtin.NativeObject):
1204+
destroy_value %2 : $Builtin.NativeObject
1205+
br bb3
1206+
1207+
bb2:
1208+
br bb3
1209+
1210+
bb3:
1211+
%9999 = tuple()
1212+
return %9999 : $()
1213+
}
1214+
1215+
// CHECK-LABEL: sil [ossa] @switch_enum_test_copyvalue_with_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
1216+
// CHECK-NOT: copy_value
1217+
// CHECK: } // end sil function 'switch_enum_test_copyvalue_with_default'
1218+
sil [ossa] @switch_enum_test_copyvalue_with_default : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
1219+
bb0(%0 : @guaranteed $FakeOptional<Builtin.NativeObject>):
1220+
%1 = copy_value %0 : $FakeOptional<Builtin.NativeObject>
1221+
switch_enum %1 : $FakeOptional<Builtin.NativeObject>, case #FakeOptional.some!enumelt.1: bb1, default bb2
1222+
1223+
bb1(%2 : @owned $Builtin.NativeObject):
1224+
destroy_value %2 : $Builtin.NativeObject
1225+
br bb3
1226+
1227+
bb2(%3 : @owned $FakeOptional<Builtin.NativeObject>):
1228+
destroy_value %3 : $FakeOptional<Builtin.NativeObject>
1229+
br bb3
1230+
1231+
bb3:
1232+
%9999 = tuple()
1233+
return %9999 : $()
1234+
}
1235+
1236+
// CHECK-LABEL: sil [ossa] @switch_enum_test_copyvalue_with_default_and_extract : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
1237+
// CHECK-NOT: copy_value
1238+
// CHECK: } // end sil function 'switch_enum_test_copyvalue_with_default_and_extract'
1239+
sil [ossa] @switch_enum_test_copyvalue_with_default_and_extract : $@convention(thin) (@guaranteed FakeOptional<Builtin.NativeObject>) -> () {
1240+
bb0(%0 : @guaranteed $FakeOptional<Builtin.NativeObject>):
1241+
%f = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1242+
%1 = copy_value %0 : $FakeOptional<Builtin.NativeObject>
1243+
switch_enum %1 : $FakeOptional<Builtin.NativeObject>, case #FakeOptional.some!enumelt.1: bb1, default bb2
1244+
1245+
bb1(%2 : @owned $Builtin.NativeObject):
1246+
destroy_value %2 : $Builtin.NativeObject
1247+
br bb3
1248+
1249+
bb2(%3 : @owned $FakeOptional<Builtin.NativeObject>):
1250+
%3a = unchecked_enum_data %3 : $FakeOptional<Builtin.NativeObject>, #FakeOptional.some!enumelt.1
1251+
apply %f(%3a) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1252+
destroy_value %3a : $Builtin.NativeObject
1253+
br bb3
1254+
1255+
bb3:
1256+
%9999 = tuple()
1257+
return %9999 : $()
1258+
}
1259+
1260+
// TODO: We currently are unable to get rid of the begin_borrow. We should be
1261+
// able to with appropriate analysis.
1262+
// CHECK-LABEL: sil [ossa] @switch_enum_test_copyvalue_with_borrow : $@convention(thin) (@owned FakeOptional<Builtin.NativeObject>) -> () {
1263+
// CHECK-NOT: copy_value
1264+
// CHECK: } // end sil function 'switch_enum_test_copyvalue_with_borrow'
1265+
sil [ossa] @switch_enum_test_copyvalue_with_borrow : $@convention(thin) (@owned FakeOptional<Builtin.NativeObject>) -> () {
1266+
bb0(%0 : @owned $FakeOptional<Builtin.NativeObject>):
1267+
%f = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1268+
%0a = begin_borrow %0 : $FakeOptional<Builtin.NativeObject>
1269+
%1 = copy_value %0a : $FakeOptional<Builtin.NativeObject>
1270+
switch_enum %1 : $FakeOptional<Builtin.NativeObject>, case #FakeOptional.some!enumelt.1: bb1, default bb2
1271+
1272+
bb1(%2 : @owned $Builtin.NativeObject):
1273+
destroy_value %2 : $Builtin.NativeObject
1274+
br bb3
1275+
1276+
bb2(%3 : @owned $FakeOptional<Builtin.NativeObject>):
1277+
%3a = unchecked_enum_data %3 : $FakeOptional<Builtin.NativeObject>, #FakeOptional.some!enumelt.1
1278+
apply %f(%3a) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1279+
destroy_value %3a : $Builtin.NativeObject
1280+
br bb3
1281+
1282+
bb3:
1283+
end_borrow %0a : $FakeOptional<Builtin.NativeObject>
1284+
destroy_value %0 : $FakeOptional<Builtin.NativeObject>
1285+
%9999 = tuple()
1286+
return %9999 : $()
1287+
}

0 commit comments

Comments
 (0)