Skip to content

Commit 0537fef

Browse files
committed
[SILCombine] Bail load[copy](uteda) if move-only.
The pass rewrites `unchecked_take_enum_data_addr` whose uses are all loads as loads--with the same ownership--of the base followed by `unchecked_enum_data`s. If the base is noncopyable and any of the loads are `load [copy]`, performing this transformation would result in a copy of that noncopyable base, which is invalid. So bail if the base is noncopyable and any of the loads are `load [copy]`s. rdar://156543855
1 parent 439afd6 commit 0537fef

File tree

2 files changed

+59
-0
lines changed

2 files changed

+59
-0
lines changed

lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,12 +1118,17 @@ SILInstruction *SILCombiner::visitUncheckedTakeEnumDataAddrInst(
11181118
return nullptr;
11191119

11201120
bool onlyLoads = true;
1121+
bool anyLoadCopies = false;
11211122
bool onlyDestroys = true;
11221123
for (auto U : getNonDebugUses(tedai)) {
11231124
// Check if it is load. If it is not a load, bail...
11241125
if (!isa<LoadInst>(U->getUser()) && !isa<LoadBorrowInst>(U->getUser()))
11251126
onlyLoads = false;
11261127

1128+
if (auto *li = dyn_cast<LoadInst>(U->getUser()))
1129+
if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Copy)
1130+
anyLoadCopies = true;
1131+
11271132
// If we have a load_borrow, perform an additional check that we do not have
11281133
// any reborrow uses. We do not handle reborrows in this optimization.
11291134
if (auto *lbi = dyn_cast<LoadBorrowInst>(U->getUser())) {
@@ -1166,6 +1171,11 @@ SILInstruction *SILCombiner::visitUncheckedTakeEnumDataAddrInst(
11661171
if (tedai->getOperand()->getType().isAddressOnly(*tedai->getFunction()))
11671172
return nullptr;
11681173

1174+
// If the enum is noncopyable and any loads cause copies, the transformation
1175+
// would be illegal because it would introduce a copy of the noncopyable enum.
1176+
if (tedai->getOperand()->getType().isMoveOnly() && anyLoadCopies)
1177+
return nullptr;
1178+
11691179
// Grab the EnumAddr.
11701180
SILLocation loc = tedai->getLoc();
11711181
Builder.setCurrentDebugScope(tedai->getDebugScope());

test/SILOptimizer/sil_combine_ossa.sil

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,14 @@ struct MoveOnlyStruct: ~Copyable {
102102
var value: MyInt
103103
}
104104

105+
enum NoncopyableEnum : ~Copyable {
106+
case i(CopyableStruct)
107+
}
108+
109+
struct CopyableStruct {
110+
var guts: AnyObject
111+
}
112+
105113
//////////////////////
106114
// Simple DCE Tests //
107115
//////////////////////
@@ -5530,3 +5538,44 @@ bb0:
55305538
return %6
55315539
}
55325540

5541+
// unchecked_take_enum_data_addr of a noncopyable base must not be combined if
5542+
// there are any load [copy] users.
5543+
// CHECK-LABEL: sil [ossa] @no_combine_uteda_load_copy_noncopyable {{.*}} {
5544+
// CHECK: unchecked_take_enum_data_addr
5545+
// CHECK-LABEL: } // end sil function 'no_combine_uteda_load_copy_noncopyable'
5546+
sil [ossa] @no_combine_uteda_load_copy_noncopyable : $@convention(thin) (@in NoncopyableEnum) -> () {
5547+
entry(%e_addr : $*NoncopyableEnum):
5548+
%i_addr = unchecked_take_enum_data_addr %e_addr : $*NoncopyableEnum, #NoncopyableEnum.i!enumelt
5549+
%i_copy = load [copy] %i_addr : $*CopyableStruct
5550+
apply undef(%i_copy) : $@convention(thin) (@owned CopyableStruct) -> ()
5551+
%i = load [take] %i_addr : $*CopyableStruct
5552+
apply undef(%i) : $@convention(thin) (@owned CopyableStruct) -> ()
5553+
return undef : $()
5554+
}
5555+
5556+
// unchecked_take_enum_data_addr of a noncopyable base should be combined if
5557+
// there are only load [take] users.
5558+
// CHECK-LABEL: sil [ossa] @combine_uteda_load_take_noncopyable {{.*}} {
5559+
// CHECK-NOT: unchecked_take_enum_data_addr
5560+
// CHECK-LABEL: } // end sil function 'combine_uteda_load_take_noncopyable'
5561+
sil [ossa] @combine_uteda_load_take_noncopyable : $@convention(thin) (@in NoncopyableEnum) -> () {
5562+
entry(%e_addr : $*NoncopyableEnum):
5563+
%i_addr = unchecked_take_enum_data_addr %e_addr : $*NoncopyableEnum, #NoncopyableEnum.i!enumelt
5564+
%i = load [take] %i_addr : $*CopyableStruct
5565+
apply undef(%i) : $@convention(thin) (@owned CopyableStruct) -> ()
5566+
return undef : $()
5567+
}
5568+
5569+
// unchecked_take_enum_data_addr of a noncopyable base should be combined if
5570+
// there are only load_borrow users.
5571+
// CHECK-LABEL: sil [ossa] @combine_uteda_load_borrow_noncopyable {{.*}} {
5572+
// CHECK-NOT: unchecked_take_enum_data_addr
5573+
// CHECK-LABEL: } // end sil function 'combine_uteda_load_borrow_noncopyable'
5574+
sil [ossa] @combine_uteda_load_borrow_noncopyable : $@convention(thin) (@in_guaranteed NoncopyableEnum) -> () {
5575+
entry(%e_addr : $*NoncopyableEnum):
5576+
%i_addr = unchecked_take_enum_data_addr %e_addr : $*NoncopyableEnum, #NoncopyableEnum.i!enumelt
5577+
%i = load_borrow %i_addr : $*CopyableStruct
5578+
apply undef(%i) : $@convention(thin) (@guaranteed CopyableStruct) -> ()
5579+
end_borrow %i : $CopyableStruct
5580+
return undef : $()
5581+
}

0 commit comments

Comments
 (0)