Skip to content

Commit 64d091d

Browse files
eecksteinatrick
authored andcommitted
AliasAnalysis: consider a take from an address as an write
Currently we only have load [take] in OSSA which needed to be changed. (copy_addr is not handled in MemBehavior at all, yet) Even if the memory is physically not modified, conceptually it's "destroyed" when the value is taken. Optimizations, like TempRValueOpt rely on this behavior when the check for may-writes. This fixes a MemoryLifetime failure in TempRValueOpt. (cherry picked from commit 547d527)
1 parent ed2318f commit 64d091d

File tree

4 files changed

+52
-0
lines changed

4 files changed

+52
-0
lines changed

include/swift/SIL/SILInstruction.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,12 @@ class SILInstruction
375375
/// The instruction may read memory.
376376
MayRead,
377377
/// The instruction may write to memory.
378+
/// This includes destroying or taking from memory (e.g. destroy_addr,
379+
/// copy_addr [take], load [take]).
380+
/// Although, physically, destroying or taking does not modify the memory,
381+
/// it is important to model it is a write. Optimizations must not assume
382+
/// that the value stored in memory is still available for loading after
383+
/// the memory is destroyed or taken.
378384
MayWrite,
379385
/// The instruction may read or write memory.
380386
MayReadWrite,

lib/SILOptimizer/Analysis/MemoryBehavior.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,10 @@ MemBehavior MemoryBehaviorVisitor::visitLoadInst(LoadInst *LI) {
243243
if (!mayAlias(LI->getOperand()))
244244
return MemBehavior::None;
245245

246+
// A take is modelled as a write. See MemoryBehavior::MayWrite.
247+
if (LI->getOwnershipQualifier() == LoadOwnershipQualifier::Take)
248+
return MemBehavior::MayReadWrite;
249+
246250
LLVM_DEBUG(llvm::dbgs() << " Could not prove that load inst does not alias "
247251
"pointer. Returning may read.\n");
248252
return MemBehavior::MayRead;

test/SILOptimizer/mem-behavior-all.sil

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,21 @@ bb0(%0 : $*Builtin.Int32):
113113
return %val : $Builtin.Int32
114114
}
115115

116+
// CHECK-LABEL: @testLoadTake
117+
// CHECK: PAIR #0.
118+
// CHECK-NEXT: %2 = load [take] %0 : $*C
119+
// CHECK-NEXT: %0 = argument of bb0 : $*C
120+
// CHECK-NEXT: r=1,w=1,se=1
121+
// CHECK: PAIR #1.
122+
// CHECK-NEXT: %2 = load [take] %0 : $*C
123+
// CHECK-NEXT: %1 = argument of bb0 : $*C
124+
// CHECK-NEXT: r=0,w=0,se=0
125+
sil [ossa] @testLoadTake : $@convention(thin) (@in C, @in_guaranteed C) -> @owned C {
126+
bb0(%0 : $*C, %1 : $*C):
127+
%2 = load [take] %0 : $*C
128+
return %2 : $C
129+
}
130+
116131
// ===-----------------------------------------------------------------------===
117132
// Test the effect of a [deinit] access on a global 'let'
118133
//

test/SILOptimizer/temp_rvalue_opt.sil

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ struct GS<Base> {
1616

1717
class Klass {}
1818

19+
struct Str {
20+
var kl: Klass
21+
var _value: Builtin.Int64
22+
}
23+
1924
sil @unknown : $@convention(thin) () -> ()
2025

2126
sil @inguaranteed_user_without_result : $@convention(thin) (@in_guaranteed Klass) -> () {
@@ -33,6 +38,8 @@ bb0(%0 : $*Klass, %1 : $*Klass):
3338

3439
sil @throwing_function : $@convention(thin) (@in_guaranteed Klass) -> ((), @error Error)
3540

41+
sil [ossa] @takeStr : $@convention(thin) (@owned Str) -> ()
42+
3643
///////////
3744
// Tests //
3845
///////////
@@ -124,6 +131,26 @@ bb0(%0 : $*B, %1 : $*GS<B>):
124131
return %9999 : $()
125132
}
126133

134+
// Check that we don't cause a memory lifetime failure with load [take]
135+
136+
// CHECK-LABEL: sil [ossa] @take_from_source
137+
// CHECK: alloc_stack
138+
// CHECK: copy_addr
139+
// CHECK: load
140+
// CHECK: load
141+
// CHECK: } // end sil function 'take_from_source'
142+
sil [ossa] @take_from_source : $@convention(thin) (@in Str) -> @owned Str {
143+
bb0(%0 : $*Str):
144+
%1 = alloc_stack $Str
145+
copy_addr %0 to [initialization] %1 : $*Str
146+
%3 = load [take] %0 : $*Str
147+
%4 = load [copy] %1 : $*Str
148+
%f = function_ref @takeStr : $@convention(thin) (@owned Str) -> ()
149+
%a = apply %f(%4) : $@convention(thin) (@owned Str) -> ()
150+
destroy_addr %1 : $*Str
151+
dealloc_stack %1 : $*Str
152+
return %3 : $Str
153+
}
127154
// CHECK-LABEL: sil @load_in_wrong_block
128155
// CHECK: bb0(%0 : $*GS<B>):
129156
// CHECK-NEXT: alloc_stack

0 commit comments

Comments
 (0)