[PATCH] D17212: [ThinLTO] Support for call graph in per-module and combined summary.

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:2486
@@ +2485,3 @@
+    if (!isa<GlobalValue>(U2)) {
+      findRefEdges(U2, VE, RefEdges, Visited);
+      continue;
----------------
joker.eph wrote:
> tejohnson wrote:
> > joker.eph wrote:
> > > tejohnson wrote:
> > > > joker.eph wrote:
> > > > > Guarantee on the recursion depth?
> > > > Beyond the Visited set check earlier? What would you like to see?
> > > I was not worried on non-termination or corretness, but depth that would explode the stack.
> > > 
> > > It was late and I couldn't think clearly enough to find the answer myself. I just got a coffee so I should be able to answer myself now ;)
> > > 
> > > -> For every instruction you will pull transitively all the operands until you reach a global (or something already seen). i.e. this is a DFS search on the SSA graph. 
> > > If I'm correct, a worklist would probably be appropriate here.
> > >  
> > I wouldn't have thought that an instruction would be large enough to cause stack explosion. But I have no issue with changing this to a worklist iteration instead.
> I may misunderstand, but it seems to me that the depth is not the width of a single instruction, but potentially on the order of the number of instructions in the Function.
> If I'm wrong and you're bounded somehow by the number of operands, then recursion is fine with me.
If you are right then this needs to change beyond moving to a worklist, as I only intended to capture the references within a single instruction or variable def.

The two entry points to this routine are in WriteFunction, where we pass in an Instruction as the User, and in WriteModuleLevelReferences, where we pass in a GlobalVariable (and I had confirmed that the operands of a variable are its initializer). We recursively walk the given User's operands(). I wouldn't have thought that we could jump from a given Instruction to other instructions in the function this way?


http://reviews.llvm.org/D17212