Skip to content

Commit 95f43bc

Browse files
committed
Fix a bug in SILValue's hoistAddressProjections
We have to insert instructions that are operands *before* the instruction using them as an operand. Also fix a bug in the same code when looking through single predecessor basic block arguments. Add a testcase for all three cases. rdar://23159379
1 parent d326c8e commit 95f43bc

File tree

2 files changed

+94
-1
lines changed

2 files changed

+94
-1
lines changed

lib/SIL/SILValue.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ void Operand::hoistAddressProjections(SILInstruction *InsertBefore,
205205
DominanceInfo *DomTree) {
206206
SILValue V = get();
207207
SILInstruction *Prev = nullptr;
208+
auto *InsertPt = InsertBefore;
208209
while (true) {
209210
SILValue Incoming = stripSinglePredecessorArgs(V);
210211

@@ -214,11 +215,13 @@ void Operand::hoistAddressProjections(SILInstruction *InsertBefore,
214215
// If we are the operand itself set the operand to the incoming
215216
// arugment.
216217
set(Incoming);
218+
V = Incoming;
217219
} else {
218220
// Otherwise, set the previous projections operand to the incoming
219221
// argument.
220222
assert(Prev && "Must have seen a projection");
221223
Prev->setOperand(0, Incoming);
224+
V = Incoming;
222225
}
223226
}
224227

@@ -234,7 +237,8 @@ void Operand::hoistAddressProjections(SILInstruction *InsertBefore,
234237

235238
// Move the current projection and memorize it for the next iteration.
236239
Prev = Inst;
237-
Inst->moveBefore(InsertBefore);
240+
Inst->moveBefore(InsertPt);
241+
InsertPt = Inst;
238242
V = Inst->getOperand(0);
239243
continue;
240244
}

test/SILPasses/cowarray_opt.sil

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ class MyArrayContainer<T> {
2727
deinit
2828
}
2929

30+
struct Container {
31+
var array: MyArray<MyStruct>
32+
}
33+
34+
struct ContainerContainer {
35+
var container: Container
36+
}
37+
3038
sil [_semantics "array.make_mutable"] @array_make_mutable : $@convention(method) (@inout MyArray<MyStruct>) -> ()
3139
sil [_semantics "array.get_count"] @guaranteed_array_get_count : $@convention(method) (@guaranteed MyArray<MyStruct>) -> Int
3240
sil [_semantics "array.get_capacity"] @guaranteed_array_get_capacity : $@convention(method) (@guaranteed MyArray<MyStruct>) -> Int
@@ -1013,3 +1021,84 @@ bb6(%23 : $Builtin.Int64):
10131021
%73 = apply %72() : $@convention(thin) () -> ()
10141022
cond_br undef, bb5, bb6(%26 : $Builtin.Int64)
10151023
}
1024+
1025+
// CHECK-LABEL: sil @hoist_projections
1026+
// CHECK: bb0([[CONTAINER:%[0-9]+]]
1027+
// CHECK: [[CONTAINER2:%.*]] = struct_element_addr [[CONTAINER]] : $*ContainerContainer
1028+
// CHECK: [[ARRAY:%.*]] = struct_element_addr [[CONTAINER2]] : $*Container,
1029+
// CHECK: [[FUN:%[0-9]+]] = function_ref @array_make_mutable
1030+
// CHECK: apply [[FUN]]([[ARRAY]]
1031+
// CHECK: bb1
1032+
// CHECK-NOT: array_make_mutable
1033+
// CHECK-NOT: apply [[FUN]]
1034+
sil @hoist_projections : $@convention(thin) (@inout ContainerContainer, @inout Builtin.Int1) -> () {
1035+
bb0(%0 : $*ContainerContainer, %1 : $*Builtin.Int1):
1036+
br bb1
1037+
1038+
bb1:
1039+
%2 = struct_element_addr %0 : $*ContainerContainer, #ContainerContainer.container
1040+
br bb3(%2 : $*Container)
1041+
1042+
bb3(%3: $*Container):
1043+
%4 = struct_element_addr %3 : $*Container, #Container.array
1044+
%5 = function_ref @array_make_mutable : $@convention(method) (@inout MyArray<MyStruct>) -> ()
1045+
%6 = apply %5(%4) : $@convention(method) (@inout MyArray<MyStruct>) -> ()
1046+
cond_br undef, bb1, bb2
1047+
1048+
bb2:
1049+
%7 = tuple()
1050+
return %7 : $()
1051+
}
1052+
1053+
// CHECK-LABEL: sil @hoist_projections2
1054+
// CHECK: bb0([[CONTAINER:%[0-9]+]]
1055+
// CHECK: [[CONTAINER2:%.*]] = struct_element_addr [[CONTAINER]] : $*ContainerContainer
1056+
// CHECK: [[ARRAY:%.*]] = struct_element_addr [[CONTAINER2]] : $*Container,
1057+
// CHECK: [[FUN:%[0-9]+]] = function_ref @array_make_mutable
1058+
// CHECK: apply [[FUN]]([[ARRAY]]
1059+
// CHECK: bb1
1060+
// CHECK-NOT: array_make_mutable
1061+
// CHECK-NOT: apply [[FUN]]
1062+
sil @hoist_projections2 : $@convention(thin) (@inout ContainerContainer, @inout Builtin.Int1) -> () {
1063+
bb0(%0 : $*ContainerContainer, %1 : $*Builtin.Int1):
1064+
br bb1
1065+
1066+
bb1:
1067+
%2 = struct_element_addr %0 : $*ContainerContainer, #ContainerContainer.container
1068+
%3 = struct_element_addr %2 : $*Container, #Container.array
1069+
br bb3(%3 : $*MyArray<MyStruct>)
1070+
1071+
bb3(%4 : $*MyArray<MyStruct>):
1072+
%5 = function_ref @array_make_mutable : $@convention(method) (@inout MyArray<MyStruct>) -> ()
1073+
%6 = apply %5(%4) : $@convention(method) (@inout MyArray<MyStruct>) -> ()
1074+
cond_br undef, bb1, bb2
1075+
1076+
bb2:
1077+
%7 = tuple()
1078+
return %7 : $()
1079+
}
1080+
1081+
// CHECK-LABEL: sil @hoist_projections3
1082+
// CHECK: bb0([[CONTAINER:%[0-9]+]]
1083+
// CHECK: [[CONTAINER2:%.*]] = struct_element_addr [[CONTAINER]] : $*ContainerContainer
1084+
// CHECK: [[ARRAY:%.*]] = struct_element_addr [[CONTAINER2]] : $*Container,
1085+
// CHECK: [[FUN:%[0-9]+]] = function_ref @array_make_mutable
1086+
// CHECK: apply [[FUN]]([[ARRAY]]
1087+
// CHECK: bb1
1088+
// CHECK-NOT: array_make_mutable
1089+
// CHECK-NOT: apply [[FUN]]
1090+
sil @hoist_projections3 : $@convention(thin) (@inout ContainerContainer, @inout Builtin.Int1) -> () {
1091+
bb0(%0 : $*ContainerContainer, %1 : $*Builtin.Int1):
1092+
br bb1
1093+
1094+
bb1:
1095+
%2 = struct_element_addr %0 : $*ContainerContainer, #ContainerContainer.container
1096+
%3 = struct_element_addr %2 : $*Container, #Container.array
1097+
%5 = function_ref @array_make_mutable : $@convention(method) (@inout MyArray<MyStruct>) -> ()
1098+
%6 = apply %5(%3) : $@convention(method) (@inout MyArray<MyStruct>) -> ()
1099+
cond_br undef, bb1, bb2
1100+
1101+
bb2:
1102+
%7 = tuple()
1103+
return %7 : $()
1104+
}

0 commit comments

Comments
 (0)