[LLVMdev] Preserving ProfileInfo in several Passes

[Andreas Neustifter]

Hi!

On 12/03/2009 07:50 PM, Dan Gohman wrote:
 > Hello,
 >
 > Here are a few misc. comments on this patch.
 >
 > Would it make sense to mark the ProfileInfo passes as "CFGOnly?"
 > If so, that would let them be automatically preserved by passes
 > which don't modify the CFG (and that call AU.setPreservesCFG()).

Yes, it would, how do I do this? :-)

 >> +  if (ProfileInfo* PI = getAnalysisIfAvailable<ProfileInfo>()) {
 >> +    PI->splitEdge(OrigPreHeader, NewHeader, NewPreHeader);
 >> +  }
 >> +
 >>     // Preserve canonical loop form, which means Exit block should
 >>     // have only one predecessor.
 >>     SplitEdge(L->getLoopLatch(), Exit, this);
 >
 > Would it make sense to move the ProfileInfo updating code into
 > SplitEdge? That way all users of SplitEdge would automatically do
 > the right thing. Actually, SplitEdge just delegates to either
 > SplitCriticalEdge or SplitBlock, so those two should do the work.

Yes, this is implemented already, but I have not removed the call to 
PI->SplitEdge(), in generall this patch is perliminary and my big 
question was how to deal with it commiting-wise, I still have to figure 
out some technical details. But thanks for the hint!

 > In TailRecursionElimination.cpp, there's logic which uses a
 > "10-fold increase" heuristic. It would be nice if that code were
 > factored out into a utility routine so that the profiling
 > heuristics are more centralized.

Agreed. Its hard to find all points that use such heuristics, since I 
couldn't find a common wording to grep on...

 >> -        bool Folded = ConstantFoldTerminator(I->getParent());
 >> +        bool Folded = ConstantFoldTerminator(I->getParent(), this);
 >
 > I don't see any corresponding changes to the definition of
 > ConstantFoldTerminator. Are there files missing from the patch?

As mentioned earlier, I was more interested in the "HowTo" of handling 
this type of changes. But yes, the include/* changes are missing from 
this patch to keep it short and still make my point.
(Maybe this was not a good idea...).

Thanks for the comments!

Andi