Skip to content

Commit e1c04c9

Browse files
Merge pull request #72318 from nate-chandler/noncopyable-bugs/20240313/1
[CanonicalizeInstruction] Process instruction "at" load after rewriting.
2 parents 0a627bc + c612f3c commit e1c04c9

File tree

3 files changed

+119
-3
lines changed

3 files changed

+119
-3
lines changed

lib/SILOptimizer/Utils/CanonicalizeInstruction.cpp

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,10 @@ static void replaceUsesOfExtract(SingleValueInstruction *extract,
139139
// (struct_extract (load %base))
140140
// ->
141141
// (load (struct_element_addr %base, #field)
142-
//
143-
// TODO: Consider handling LoadBorrowInst.
144142
static SILBasicBlock::iterator
145143
splitAggregateLoad(LoadOperation loadInst, CanonicalizeInstruction &pass) {
144+
auto *block = loadInst->getParentBlock();
145+
auto *instBeforeLoad = loadInst->getPreviousInstruction();
146146
// Keep track of the next iterator after any newly added or to-be-deleted
147147
// instructions. This must be valid regardless of whether the pass immediately
148148
// deletes the instructions or simply records them for later deletion.
@@ -338,7 +338,22 @@ splitAggregateLoad(LoadOperation loadInst, CanonicalizeInstruction &pass) {
338338
++nextII;
339339
}
340340
deleteAllDebugUses(*loadInst, pass.getCallbacks());
341-
return killInstAndIncidentalUses(*loadInst, nextII, pass);
341+
nextII = killInstAndIncidentalUses(*loadInst, nextII, pass);
342+
/// A change has been made; and the load instruction is deleted. The caller
343+
/// should now process the instruction where the load was before.
344+
///
345+
/// BEFORE TRANSFORM | AFTER TRANSFORM
346+
/// prequel_2 | prequel_2
347+
/// prequel_1 | prequel_1
348+
/// load | +-> ???
349+
/// sequel_1 | | ???
350+
/// sequel_2 | | ???
351+
/// |
352+
/// The instruction the caller should process next.
353+
if (instBeforeLoad)
354+
return instBeforeLoad->getNextInstruction()->getIterator();
355+
else
356+
return block->begin();
342357
}
343358

344359
// Given a store within a single property struct, recursively form the parent
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// RUN: %target-run-simple-swift(-Xfrontend -sil-verify-all -enable-experimental-feature MoveOnlyPartialConsumption) | %FileCheck %s
2+
// RUN: %target-run-simple-swift(-O -Xfrontend -sil-verify-all -enable-experimental-feature MoveOnlyPartialConsumption) | %FileCheck %s
3+
4+
// REQUIRES: executable_test
5+
6+
func destructure() {
7+
let q = getQuad(name: "q")
8+
9+
// CHECK: hi q.p1.u1
10+
// CHECK: hi q.p1.u2
11+
// CHECK: hi q.p2.u1
12+
// CHECK: hi q.p2.u2
13+
14+
take(q.p1.u1)
15+
// CHECK: bye q.p1.u1
16+
take(q.p1.u2)
17+
// CHECK: bye q.p1.u2
18+
take(q.p2.u1)
19+
// CHECK: bye q.p2.u1
20+
take(q.p2.u2)
21+
// CHECK: bye q.p2.u2
22+
}
23+
24+
struct Unique : ~Copyable {
25+
let name: String
26+
init(name: String) {
27+
self.name = name
28+
print("hi", name)
29+
}
30+
deinit {
31+
print("bye", name)
32+
}
33+
}
34+
35+
func take(_ u: consuming Unique) {}
36+
37+
struct Pair : ~Copyable {
38+
var u1: Unique
39+
var u2: Unique
40+
init(name: String) {
41+
self.u1 = .init(name: "\(name).u1")
42+
self.u2 = .init(name: "\(name).u2")
43+
}
44+
}
45+
46+
struct Quad : ~Copyable {
47+
var p1: Pair
48+
var p2: Pair
49+
init(name: String) {
50+
self.p1 = .init(name: "\(name).p1")
51+
self.p2 = .init(name: "\(name).p2")
52+
}
53+
}
54+
55+
func getQuad(name: String) -> Quad {
56+
return Quad(name: name)
57+
}
58+
59+
destructure()

test/SILOptimizer/silgen_cleanup.sil

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,3 +362,45 @@ bb9(%0 : @owned $Klass):
362362
destroy_value %0 : $Klass
363363
return %v : $Builtin.Int64
364364
}
365+
366+
struct Outer {
367+
var middle: Middle
368+
}
369+
370+
struct Middle {
371+
var inner: Inner
372+
}
373+
374+
struct Inner {
375+
var guts: Builtin.AnyObject
376+
}
377+
378+
sil @getOuter : $@convention(thin) () -> @owned Outer
379+
sil @takeInner : $@convention(thin) (@owned Inner) -> ()
380+
381+
// CHECK-LABEL: sil [ossa] @narrowLoadThroughProjectionSequence : {{.*}} {
382+
// CHECK: [[OUTER_ADDR:%[^,]+]] = project_box
383+
// CHECK: [[MIDDLE_ADDR:%[^,]+]] = struct_element_addr [[OUTER_ADDR]]
384+
// CHECK: [[INNER_ADDR:%[^,]+]] = struct_element_addr [[MIDDLE_ADDR]]
385+
// CHECK: [[INNER_BORROW:%[^,]+]] = load_borrow [[INNER_ADDR]]
386+
// CHECK: copy_value [[INNER_BORROW]]
387+
// CHECK-LABEL: } // end sil function 'narrowLoadThroughProjectionSequence'
388+
sil [ossa] @narrowLoadThroughProjectionSequence : $@convention(thin) () -> () {
389+
%box = alloc_box ${ let Outer }
390+
%box_borrow = begin_borrow [lexical] [var_decl] %box : ${ let Outer }
391+
%outer_addr = project_box %box_borrow : ${ let Outer }, 0
392+
%getOuter = function_ref @getOuter : $@convention(thin) () -> @owned Outer
393+
%outer = apply %getOuter() : $@convention(thin) () -> @owned Outer
394+
store %outer to [init] %outer_addr : $*Outer
395+
%outer_borrow = load_borrow %outer_addr : $*Outer
396+
%middle = struct_extract %outer_borrow : $Outer, #Outer.middle
397+
%inner = struct_extract %middle : $Middle, #Middle.inner
398+
%inner_copy = copy_value %inner : $Inner
399+
%takeInner = function_ref @takeInner : $@convention(thin) (@owned Inner) -> ()
400+
apply %takeInner(%inner_copy) : $@convention(thin) (@owned Inner) -> ()
401+
end_borrow %outer_borrow : $Outer
402+
end_borrow %box_borrow : ${ let Outer }
403+
dealloc_box %box : ${ let Outer }
404+
%retval = tuple ()
405+
return %retval : $()
406+
}

0 commit comments

Comments
 (0)