[llvm-commits] [llvm] r80566 - in /llvm/trunk: include/llvm/Analysis/CallGraph.h lib/Analysis/IPA/CallGraph.cpp lib/Analysis/IPA/CallGraphSCCPass.cpp

[Chris Lattner]

On Aug 31, 2009, at 1:31 AM, Duncan Sands wrote:
> Hi Chris, this seems like a reasonable approach.
> +  // The the function pass(es) modified the IR, they may have  
> clobbered the
> The the -> The
> +  MadeChange = MadeChange;
> Doesn't seem very useful :)

Fixed, thanks.

> +  // Scan all functions in the SCC.
>
> Can a function pass muck with CurSCC?

No, function passes are allowed to modify the body of a function, but  
they have no way to change the contents of the CurSCC vector.

>
>> +    if (F == 0 || F->isDeclaration()) continue;
>
> What about functions with weak linkage?

This is mirroring logic for the current callgraph construction stuff.   
Weak functions etc could definitely be improved in the callgraph by  
not modeling them or by treating them as external functions, but the  
callgraph and cgsccpm logic needs to be the same.

> Also, I think this routine
> should live in the same file as the other callgraph computing
> routines.  Perhaps it can be merged somehow with the existing CG
> calculation.  (That way, if someone modifies the interaction between,
> say, weak linkage and how the callgraph is calculated, this won't
> be forgotten).

Right, another day though :)

>
>> +    if (!CallGraphUpToDate)
>> +      RefreshCallGraph(CurSCC, CG);
>
> Don't you need to set CallGraphUpToDate to "true" here?

It's dead after this point.

Thanks for the review!

-Chris