Skip to content

Commit 722b92c

Browse files
RedundantLoadElim: Fix invalid destructure_struct emission without aggressive reg2mem
The `shouldExpand` in `OptUtils.swift` was incorrectly returning `true` unconditionally when `useAggressiveReg2MemForCodeSize` was disabled. The expansion might be invalid for types with addr-only types and structs with deinit, but we didn't check them before. This could lead to invalid `destructure_struct` instructions without `drop_deinit` being emitted.
1 parent 32aa4d9 commit 722b92c

File tree

3 files changed

+52
-3
lines changed

3 files changed

+52
-3
lines changed

SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,9 +1100,6 @@ extension Type {
11001100
/// False if expanding a type is invalid. For example, expanding a
11011101
/// struct-with-deinit drops the deinit.
11021102
func shouldExpand(_ context: some Context) -> Bool {
1103-
if !context.options.useAggressiveReg2MemForCodeSize {
1104-
return true
1105-
}
11061103
return context.bridgedPassContext.shouldExpand(self.bridged)
11071104
}
11081105
}

lib/SIL/Utils/InstructionUtils.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1486,10 +1486,18 @@ bool swift::shouldExpand(SILModule &module, SILType ty) {
14861486
if (nominalTy->getValueTypeDestructor())
14871487
return false;
14881488
}
1489+
1490+
// At this point we know it's valid to expand the type, next decide if we
1491+
// "should" expand it.
1492+
14891493
if (EnableExpandAll) {
14901494
return true;
14911495
}
14921496

1497+
if (!module.getOptions().UseAggressiveReg2MemForCodeSize) {
1498+
return false;
1499+
}
1500+
14931501
unsigned numFields = module.Types.countNumberOfFields(ty, expansion);
14941502
return (numFields <= 6);
14951503
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// RUN: %target-sil-opt -module-name Swift -sil-print-types -enforce-exclusivity=none -enable-sil-verify-all %s -redundant-load-elimination | %FileCheck %s
2+
// RUN: %target-sil-opt -module-name Swift -sil-print-types -enforce-exclusivity=none -enable-sil-verify-all %s -redundant-load-elimination -disable-aggressive-reg2mem | %FileCheck %s
3+
4+
// REQUIRES: swift_in_compiler
5+
6+
import Builtin
7+
8+
@_marker protocol Copyable {}
9+
10+
final class C1 {}
11+
12+
struct S1 {}
13+
14+
struct StructWithDeinit : ~Copyable {
15+
// stored class-instance to ensure this struct has a non-trivial
16+
var f1: C1
17+
var f2: S1
18+
deinit
19+
}
20+
21+
// CHECK-LABEL: sil [ossa] @test : $@convention(thin) (@owned StructWithDeinit, @owned S1) -> ()
22+
// CHECK: %[[STACK:.*]] = alloc_stack $StructWithDeinit
23+
// CHECK: load [take] %[[STACK]] : $*StructWithDeinit
24+
// CHECK: } // end sil function 'test'
25+
sil [ossa] @test : $@convention(thin) (@owned StructWithDeinit, @owned S1) -> () {
26+
bb0(%0 : @owned $StructWithDeinit, %1 : $S1):
27+
%2 = alloc_stack $StructWithDeinit
28+
store %0 to [init] %2
29+
// partial store into f2
30+
%3 = struct_element_addr %2, #StructWithDeinit.f2
31+
store %1 to [trivial] %3
32+
33+
// redundant load of the whole struct
34+
%4 = load [take] %2
35+
destroy_value %4
36+
37+
dealloc_stack %2
38+
%5 = tuple ()
39+
return %5
40+
}
41+
42+
sil @s1_deinit : $@convention(method) (@owned StructWithDeinit) -> ()
43+
sil_moveonlydeinit StructWithDeinit { @s1_deinit }
44+

0 commit comments

Comments
 (0)