Skip to content

Commit 2584078

Browse files
committed
MergeFunctions: handle self recursive functions correctly.
First, it fixes a crash where the eliminated function is still referenced. This shows up if two equivalent self-recursive functions are merged and those functions are internal. Fixes SR-4514, rdar://problem/31479425 Second, it avoids creating a not needed parameter for really equivalent self recursive functions.
1 parent 763b573 commit 2584078

File tree

2 files changed

+92
-3
lines changed

2 files changed

+92
-3
lines changed

lib/LLVMPasses/LLVMMergeFunctions.cpp

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -782,20 +782,31 @@ bool SwiftMergeFunctions::deriveParams(ParamInfos &Params,
782782

783783
/// Returns true if the \p OpIdx's constant operand in the current instruction
784784
/// does differ in any of the functions in \p FInfos.
785+
///
786+
/// But it accepts the case where all operands refer to their containing
787+
/// functions (in case of self recursive functions).
785788
bool SwiftMergeFunctions::constsDiffer(const FunctionInfos &FInfos,
786789
unsigned OpIdx) {
787790
Constant *CommonConst = nullptr;
791+
bool matching = true;
792+
bool selfReferencing = true;
788793

789794
for (const FunctionInfo &FI : FInfos) {
790795
Value *Op = FI.CurrentInst->getOperand(OpIdx);
791796
if (Constant *C = dyn_cast<Constant>(Op)) {
797+
if (C != FI.F)
798+
selfReferencing = false;
799+
792800
if (!CommonConst) {
793801
CommonConst = C;
794802
} else if (C != CommonConst) {
795-
return true;
803+
matching = false;
796804
}
805+
if (!selfReferencing && !matching)
806+
return true;
797807
}
798808
}
809+
assert(selfReferencing || matching);
799810
return false;
800811
}
801812

@@ -903,6 +914,7 @@ void SwiftMergeFunctions::mergeWithParams(const FunctionInfos &FInfos,
903914
assert(!isInEquivalenceClass(&*Iter->second));
904915
Iter->second->F = nullptr;
905916
FuncEntries.erase(Iter);
917+
DEBUG(dbgs() << " Erase " << OrigFunc->getName() << '\n');
906918
OrigFunc->eraseFromParent();
907919
} else {
908920
// Otherwise we need a thunk which calls the merged function.
@@ -1062,8 +1074,15 @@ bool SwiftMergeFunctions::replaceDirectCallers(Function *Old, Function *New,
10621074
// Add the new parameters.
10631075
for (const ParamInfo &PI : Params) {
10641076
assert(ParamIdx < NewFuncTy->getNumParams());
1065-
NewArgs.push_back(PI.Values[FuncIdx]);
1066-
OldParamTypes.push_back(PI.Values[FuncIdx]->getType());
1077+
Constant *ArgValue = PI.Values[FuncIdx];
1078+
1079+
// Check if it's a self referencing function. We must not use the old
1080+
// function pointer as argument.
1081+
if (ArgValue == Old)
1082+
ArgValue = New;
1083+
1084+
NewArgs.push_back(ArgValue);
1085+
OldParamTypes.push_back(ArgValue->getType());
10671086
++ParamIdx;
10681087
}
10691088

@@ -1080,6 +1099,7 @@ bool SwiftMergeFunctions::replaceDirectCallers(Function *Old, Function *New,
10801099
CI->replaceAllUsesWith(NewCI);
10811100
CI->eraseFromParent();
10821101
}
1102+
assert(Old->use_empty() && "should have replaced all uses of old function");
10831103
return Old->hasLocalLinkage();
10841104
}
10851105

test/LLVMPasses/merge_func.ll

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,3 +323,72 @@ done:
323323
; CHECK: ret i1
324324
ret i1 %result
325325
}
326+
327+
; Check self recursive functions
328+
329+
; CHECK-NOT: @recursive1(
330+
define internal void @recursive1(i32 %x, i32 %y) {
331+
br i1 undef, label %bb1, label %bb2
332+
333+
bb1:
334+
call void @recursive1(i32 %x, i32 %y)
335+
br label %bb2
336+
337+
bb2:
338+
ret void
339+
}
340+
341+
; CHECK-LABEL: define internal void @recursive1_merged(i32, i32)
342+
; CHECK: call void @recursive1_merged(i32 %0, i32 %1)
343+
; CHECK: ret void
344+
define internal void @recursive2(i32 %x, i32 %y) {
345+
br i1 undef, label %bb1, label %bb2
346+
347+
bb1:
348+
call void @recursive2(i32 %x, i32 %y)
349+
br label %bb2
350+
351+
bb2:
352+
ret void
353+
}
354+
355+
; CHECK-LABEL: define internal void @another_recursive_func_merged(i32, void (i32)*)
356+
; CHECK: call void %1(i32 %0)
357+
; CHECK: ret void
358+
define internal void @another_recursive_func(i32 %x) {
359+
br i1 undef, label %bb1, label %bb2
360+
361+
bb1:
362+
call void @another_recursive_func(i32 %x)
363+
br label %bb2
364+
365+
bb2:
366+
ret void
367+
}
368+
369+
; CHECK-NOT: @not_really_recursive(
370+
define internal void @not_really_recursive(i32 %x) {
371+
br i1 undef, label %bb1, label %bb2
372+
373+
bb1:
374+
call void @callee1(i32 %x)
375+
br label %bb2
376+
377+
bb2:
378+
ret void
379+
}
380+
381+
; CHECK-LABEL: define void @call_recursive_funcs(i32 %x)
382+
; CHECK: call void @recursive1_merged(i32 %x, i32 %x)
383+
; CHECK: call void @recursive1_merged(i32 %x, i32 %x)
384+
; CHECK: call void bitcast (void (i32, void (i32)*)* @another_recursive_func_merged to void (i32, void (i32, void (i32)*)*)*)(i32 %x, void (i32, void (i32)*)* @another_recursive_func_merged)
385+
; CHECK: call void @another_recursive_func_merged(i32 %x, void (i32)* @callee1)
386+
; CHECK: ret void
387+
define void @call_recursive_funcs(i32 %x) {
388+
call void @recursive1(i32 %x, i32 %x)
389+
call void @recursive2(i32 %x, i32 %x)
390+
call void @another_recursive_func(i32 %x)
391+
call void @not_really_recursive(i32 %x)
392+
ret void
393+
}
394+

0 commit comments

Comments
 (0)