Improve RefreshCallGraph to remove invalid call graph edge.

[• hulx2000]

Summary

This patch is to improve CGPassManager::RefreshCallGraph to remove an invalid call graph edge corrupted by Instruction Combiner, triggered an assert in CallGraphNode::addCalledFunction later.

With commit d8214db0868f228026fa6ab1d5ca6c76ab3b5ce0, llvm's Instruction Combiner now can turn an sqrtl into a llvm.fabs.f64, the call graph edge (to be more precise, one Entry in B's CalledFunctions list) originally representing call to sqrtl now becomes invalid: the instruction is the call instruction for llvm.fabs.f64 (done by Value:: replaceAllUsesWith(..)), and the call graph node is still sqrtl, note that intrinsic shouldn't be in call graph, and call graph node for sqrtl should be gone too.

The problem can only be reproduced if we meet the all following conditions:

1. Compile the program with NDK sysroot, and program need to be complicate enough to invoke CGPassManager::RunAllPassesOnSCC.
2. For program with function A and B, A calls B, B calls some other functions, B need to be compiled before A to reproduce the problem.
3. While compiling function B, inlining is invoked and instruction combiner is able to turn a call to sqrtl into llvm.fabs.f64
4. Inlining is invoked while compiling function A (a bigger inlining threshold is needed)

This problem can't be reproduced with llvm trunk, because it doesn't turn a sqrtl into llvm.fabs.f64, but I think it is a real problem.

[• hulx2000]

ping

[apazos]

I would like to commit this bug fix today.

Chandler and other reviewers, are you ok with this change?

http://reviews.llvm.org/D7705

Ana.

[chandlerc]

Sorry for the delay reviewing this. LGTM, and yea, I can imagine it would be hard to craft a test case.

[• hulx2000]

chad merge it under commit 2fbccf6972c1468e3d252d2d3125403c4065ffff

[• hulx2000]

sorry, didn't notice the status change until now.

lib/Analysis/IPA/CallGraphSCCPass.cpp

           // list of the same call.
           CallSites.count(I->first) ||
 
-          // If the call edge is not from a call or invoke, then the function
-          // pass RAUW'd a call with another value.  This can happen when
-          // constant folding happens of well known functions etc.
-          !CallSite(I->first)) {
+          // If the call edge is not from a call or invoke, or it is a
+          // instrinsic call, then the function pass RAUW'd a call with 
+          // another value. This can happen when constant folding happens
+          // of well known functions etc.
+          !CallSite(I->first) ||
+           (CallSite(I->first).getCalledFunction() &&
+            CallSite(I->first).getCalledFunction()->isIntrinsic())) {
         assert(!CheckingMode &&
                "CallGraphSCCPass did not update the CallGraph correctly!");