Skip to content

Commit 7c75285

Browse files
committed
Fix a miscompile in the swift function merging pass.
In rare corner cases the pass merged two functions which contain incompatible call instructions. See source comment in the change for details. rdar://problem/43051718
1 parent a710c2f commit 7c75285

File tree

2 files changed

+120
-0
lines changed

2 files changed

+120
-0
lines changed

lib/LLVMPasses/LLVMMergeFunctions.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,8 @@ class SwiftMergeFunctions : public ModulePass {
402402

403403
bool deriveParams(ParamInfos &Params, FunctionInfos &FInfos);
404404

405+
bool numOperandsDiffer(FunctionInfos &FInfos);
406+
405407
bool constsDiffer(const FunctionInfos &FInfos, unsigned OpIdx);
406408

407409
bool tryMapToParameter(FunctionInfos &FInfos, unsigned OpIdx,
@@ -762,6 +764,20 @@ bool SwiftMergeFunctions::deriveParams(ParamInfos &Params,
762764
// Iterate over all instructions synchronously in all functions.
763765
do {
764766
if (isEligibleForConstantSharing(FirstFI.CurrentInst)) {
767+
768+
// Here we handle a rare corner case which needs to be explained:
769+
// Usually the number of operands match, because otherwise the functions
770+
// in FInfos would not be in the same equivalence class. There is only one
771+
// exception to that: If the current instruction is a call to a function,
772+
// which was merged in the previous iteration (in tryMergeEquivalenceClass)
773+
// then the call could be replaced and has more arguments than the
774+
// original call.
775+
if (numOperandsDiffer(FInfos)) {
776+
assert(isa<CallInst>(FirstFI.CurrentInst) &&
777+
"only calls are expected to differ in number of operands");
778+
return false;
779+
}
780+
765781
for (unsigned OpIdx = 0, NumOps = FirstFI.CurrentInst->getNumOperands();
766782
OpIdx != NumOps; ++OpIdx) {
767783

@@ -783,6 +799,16 @@ bool SwiftMergeFunctions::deriveParams(ParamInfos &Params,
783799
return true;
784800
}
785801

802+
/// Returns true if the number of operands of the current instruction differs.
803+
bool SwiftMergeFunctions::numOperandsDiffer(FunctionInfos &FInfos) {
804+
unsigned numOps = FInfos[0].CurrentInst->getNumOperands();
805+
for (const FunctionInfo &FI : ArrayRef<FunctionInfo>(FInfos).drop_front(1)) {
806+
if (FI.CurrentInst->getNumOperands() != numOps)
807+
return true;
808+
}
809+
return false;
810+
}
811+
786812
/// Returns true if the \p OpIdx's constant operand in the current instruction
787813
/// does differ in any of the functions in \p FInfos.
788814
bool SwiftMergeFunctions::constsDiffer(const FunctionInfos &FInfos,

test/LLVMPasses/merge_func.ll

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,100 @@ define i32 @func4_merged_with2(i32 %x) {
200200
}
201201

202202

203+
; The same example as above, but we cannot merge func2 with func4, because
204+
; func4 calls func1 (which is merged with func2 in the first iteration).
205+
206+
declare i32 @get_int(i32 %x)
207+
208+
; CHECK-LABEL: define i32 @Function1_merged_with_3(i32 %x)
209+
; CHECK: %1 = tail call i32 @Function1_merged_with_3Tm(i32 %x, i32* @g1)
210+
; CHECK: ret i32 %1
211+
define i32 @Function1_merged_with_3(i32 %x) {
212+
%l1 = load i32, i32* @g1, align 4
213+
%sum = add i32 %x, %l1
214+
%l2 = load i32, i32* @g2, align 4
215+
%sum2 = add i32 %sum, %l2
216+
%l3 = load i32, i32* @g3, align 4
217+
%sum3 = add i32 %sum2, %l2
218+
%l4 = load i32, i32* @g4, align 4
219+
%sum4 = add i32 %sum3, %l2
220+
%l5 = load i32, i32* @g5, align 4
221+
%sum5 = add i32 %sum4, %l2
222+
%c = call fastcc i32 @get_int(i32 %sum5)
223+
ret i32 %c
224+
}
225+
226+
; CHECK-LABEL: define i32 @Function2_not_merged(i32 %x)
227+
; CHECK: load
228+
; CHECK: load
229+
; CHECK: load
230+
; CHECK: load
231+
; CHECK: %c = call fastcc i32 @get_int
232+
; CHECK: ret i32 %c
233+
define i32 @Function2_not_merged(i32 %x) {
234+
%l1 = load i32, i32* @g2, align 4
235+
%sum = add i32 %x, %l1
236+
%l2 = load i32, i32* @g3, align 4
237+
%sum2 = add i32 %sum, %l2
238+
%l3 = load i32, i32* @g4, align 4
239+
%sum3 = add i32 %sum2, %l2
240+
%l4 = load i32, i32* @g5, align 4
241+
%sum4 = add i32 %sum3, %l2
242+
%l5 = load i32, i32* @g1, align 4
243+
%sum5 = add i32 %sum4, %l2
244+
%c = call fastcc i32 @get_int(i32 %sum5)
245+
ret i32 %c
246+
}
247+
248+
; CHECK-LABEL: define i32 @Function3_merged_with_1(i32 %x)
249+
; CHECK: %1 = tail call i32 @Function1_merged_with_3Tm(i32 %x, i32* @g2)
250+
; CHECK: ret i32 %1
251+
define i32 @Function3_merged_with_1(i32 %x) {
252+
%l1 = load i32, i32* @g2, align 4
253+
%sum = add i32 %x, %l1
254+
%l2 = load i32, i32* @g2, align 4
255+
%sum2 = add i32 %sum, %l2
256+
%l3 = load i32, i32* @g3, align 4
257+
%sum3 = add i32 %sum2, %l2
258+
%l4 = load i32, i32* @g4, align 4
259+
%sum4 = add i32 %sum3, %l2
260+
%l5 = load i32, i32* @g5, align 4
261+
%sum5 = add i32 %sum4, %l2
262+
%c = call fastcc i32 @get_int(i32 %sum5)
263+
ret i32 %c
264+
}
265+
266+
; CHECK-LABEL: define internal i32 @Function1_merged_with_3Tm(i32, i32*)
267+
; CHECK: load
268+
; CHECK: load
269+
; CHECK: load
270+
; CHECK: load
271+
; CHECK: %c = call fastcc i32 @get_int
272+
; CHECK: ret i32 %c
273+
274+
; CHECK-LABEL: define i32 @Function4_not_merged(i32 %x) {
275+
; CHECK: load
276+
; CHECK: load
277+
; CHECK: load
278+
; CHECK: load
279+
; CHECK: %1 = call fastcc i32 @Function1_merged_with_3Tm(i32 %sum5, i32* @g1)
280+
; CHECK: ret i32 %1
281+
define i32 @Function4_not_merged(i32 %x) {
282+
%l1 = load i32, i32* @g1, align 4
283+
%sum = add i32 %x, %l1
284+
%l2 = load i32, i32* @g3, align 4
285+
%sum2 = add i32 %sum, %l2
286+
%l3 = load i32, i32* @g4, align 4
287+
%sum3 = add i32 %sum2, %l2
288+
%l4 = load i32, i32* @g5, align 4
289+
%sum4 = add i32 %sum3, %l2
290+
%l5 = load i32, i32* @g1, align 4
291+
%sum5 = add i32 %sum4, %l2
292+
%c = call fastcc i32 @Function1_merged_with_3(i32 %sum5)
293+
ret i32 %c
294+
}
295+
296+
203297
; Test a call chain: caller -> callee1 -> callee2.
204298
; Functions should be merged in bottom-up order: callee2, callee1, caller.
205299
; Also check that the calling convention is preserved.

0 commit comments

Comments
 (0)