[PATCH] Update call graph after devirtualizing returned value

Pete peter_cooper at apple.com
Tue Jun 3 19:05:09 PDT 2014


================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:521-522
@@ +520,4 @@
+  CallGraphNode *CallerNode = nullptr;
+  for (auto UI = TheCall->use_begin(), UE = TheCall->use_end();
+       UI != UE;) {
+    Use &U = *UI++;
----------------
Chandler Carruth wrote:
> You can just write:
> 
>   for (Use *U : TheCall->uses()) {
Will do. At first I thought I was invalidating the iterator but actually I'm not in this case.

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:524
@@ +523,3 @@
+    Use &U = *UI++;
+    U.set(CalledF);
+    CallSite CS(U.getUser());
----------------
Chandler Carruth wrote:
> I don't understand why you're doing it this way.
> 
> I would just do the RAUW in the caller of this like normal, and then walk the uses after replacing them.
If we end up returning a Function* say, then that means walking all the users of a Function* then identifying those which are instructions in the caller. I thought that would be too slow. My current implementation is O(num return value uses).

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:526-527
@@ +525,4 @@
+    CallSite CS(U.getUser());
+    if (!CS)
+      continue;
+    if (!CallerNode) {
----------------
Chandler Carruth wrote:
> So, this misses cases where the function pointer is washed through a bitcast to i8* and back to a function pointer.
> 
> If you do the RAUW first, I think you can use instsimplify and look through casts to handle all of these cases more generally. You probably will also need to walk the uses recursively to find all the transitive users through bitcasts.
You're right. Based on how the inliner would process a call with a bit cast, I think my current code will work, but it'll be much more efficient to do what you're suggesting here.

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:529-531
@@ +528,5 @@
+    if (!CallerNode) {
+      // Lazily look up the caller node
+      const Function *Caller = TheCall->getParent()->getParent();
+      CallerNode = (*IFI.CG)[Caller];
+    }
----------------
Chandler Carruth wrote:
> Why not just pass this in?
Will do. This saved us nothing by avoiding passing it in.

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:533-534
@@ +532,4 @@
+    }
+    CallGraphNode *CalleeNode = IFI.CG->getOrInsertFunction(CalledF);
+    CallerNode->replaceCallEdge(CS, CS, CalleeNode);
+  }
----------------
Chandler Carruth wrote:
> This is a linear operation on the caller, which makes this whole thing go quadratic. Have you seen any compile time performance hits?
> 
> The entire design of the call graph makes doing this really annoying. I can't wait until we can switch to LCG....
This is stolen from the SCC pass updater which runs after inlining to recalculate the call graph. Doing it here should be performance neutral as we save the updater doing it instead.

Honestly I haven't measured the compile time impact yet. I'll get to that once we converge on a good patch.

http://reviews.llvm.org/D4012






More information about the llvm-commits mailing list