Skip to content

Commit 8090957

Browse files
committed
Fix PMO to not scalarize empty tuple
Currently PMO tries to scalarize empty tuple, and ends up deleting store of an empty tuple. This causes a cascade of problems. Mem2Reg can end up seeing loads without stores of empty tuple type, and creates undef while optimizing the alloca away. This is bad, we don't want to create undef's unnecessarily in SIL. Some optimization can queiry the function or the block on the undef leading to nullptr and a compiler crash. Fixes rdar://94829482
1 parent 4dffe27 commit 8090957

File tree

2 files changed

+44
-6
lines changed

2 files changed

+44
-6
lines changed

lib/SILOptimizer/Mandatory/PMOMemoryUseCollector.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -288,9 +288,11 @@ bool ElementUseCollector::collectUses(SILValue Pointer) {
288288
// Stores *to* the allocation are writes.
289289
if (auto *si = dyn_cast<StoreInst>(User)) {
290290
if (UI->getOperandNumber() == StoreInst::Dest) {
291-
if (PointeeType.is<TupleType>()) {
292-
UsesToScalarize.push_back(User);
293-
continue;
291+
if (auto tupleType = PointeeType.getAs<TupleType>()) {
292+
if (!tupleType->isEqual(Module.getASTContext().TheEmptyTupleType)) {
293+
UsesToScalarize.push_back(User);
294+
continue;
295+
}
294296
}
295297

296298
auto kind = ([&]() -> PMOUseKind {
@@ -322,9 +324,11 @@ bool ElementUseCollector::collectUses(SILValue Pointer) {
322324
if (auto *CAI = dyn_cast<CopyAddrInst>(User)) {
323325
// If this is a copy of a tuple, we should scalarize it so that we don't
324326
// have an access that crosses elements.
325-
if (PointeeType.is<TupleType>()) {
326-
UsesToScalarize.push_back(CAI);
327-
continue;
327+
if (auto tupleType = PointeeType.getAs<TupleType>()) {
328+
if (!tupleType->isEqual(Module.getASTContext().TheEmptyTupleType)) {
329+
UsesToScalarize.push_back(CAI);
330+
continue;
331+
}
328332
}
329333

330334
// If this is the source of the copy_addr, then this is a load. If it is

test/SILOptimizer/predictable_memopt.sil

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,40 @@ bb0(%0 : $Int):
4545
return %d : $Int
4646
}
4747

48+
sil @read_emptytuple : $@convention(thin) (@in_guaranteed ()) -> ()
49+
50+
// CHECK-LABEL: sil @emptytuple_promotion1 :
51+
// CHECK: store
52+
// CHECK-LABEL: } // end sil function 'emptytuple_promotion1'
53+
sil @emptytuple_promotion1 : $@convention(thin) () -> () {
54+
bb0:
55+
%1 = alloc_stack $()
56+
%2 = tuple ()
57+
store %2 to %1 : $*()
58+
%f = function_ref @read_emptytuple : $@convention(thin) (@in_guaranteed ()) -> ()
59+
apply %f(%1) : $@convention(thin) (@in_guaranteed ()) -> ()
60+
dealloc_stack %1 : $*()
61+
return %2 : $()
62+
}
63+
64+
// CHECK-LABEL: sil @emptytuple_promotion2 :
65+
// CHECK: store
66+
// CHECK: store
67+
// CHECK-LABEL: } // end sil function 'emptytuple_promotion2'
68+
sil @emptytuple_promotion2 : $@convention(thin) () -> () {
69+
bb0:
70+
%1 = alloc_stack $()
71+
%2 = alloc_stack $()
72+
%3 = tuple ()
73+
store %3 to %1 : $*()
74+
copy_addr %2 to %1 : $*()
75+
%f = function_ref @read_emptytuple : $@convention(thin) (@in_guaranteed ()) -> ()
76+
apply %f(%1) : $@convention(thin) (@in_guaranteed ()) -> ()
77+
dealloc_stack %2 : $*()
78+
dealloc_stack %1 : $*()
79+
return %3 : $()
80+
}
81+
4882
// In this example we create two boxes. The first box is initialized and then
4983
// taken from to initialize the second box. This means that the first box must
5084
// be dealloc_boxed (since its underlying memory is considered invalid). In

0 commit comments

Comments
 (0)