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

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 8 16:13:17 PST 2016


joker.eph added inline comments.

================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:2486
@@ +2485,3 @@
+    if (!isa<GlobalValue>(U2)) {
+      findRefEdges(U2, VE, RefEdges, Visited);
+      continue;
----------------
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.


http://reviews.llvm.org/D17212





More information about the llvm-commits mailing list