Add 'cold' attribute to functions

Diego Novillo dnovillo at google.com
Fri May 24 05:25:59 PDT 2013


On 2013-05-24 02:10 , Duncan Sands wrote:
>
> it's not necessary for correctness, it's an optimization.  It simply 
> allocates
> TI->getNumSuccessors() slots in the vector at once, rather than 
> incrementally
> resizing the vector as elements are added.  Using ->reserve doesn't 
> change the
> vector semantics in any way, for example after the ->reserve call the 
> vector
> will still report its size as being zero.  It's not clear in this case 
> if doing
> this is a win, since the TI->getNumSuccessors() elements are 
> distributed between
> two vectors not one.

Sure, I realize that.  I'm just wondering whether it'ss worth it. On the 
one hand, I don't expect the fan out of a basic block to ever be too 
large, and on the other, I expected push_back to resize the vector 
exponentially.  However, it doesn't really hurt, so I just did the change.

>
>> +  else {
>> +    // Otherwise, if the block itself contains a cold function, add 
>> it to the
>> +    // set of blocks postdominated by a cold call.
>> +    assert(!PostDominatedByColdCall.count(BB));
>> +    for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != 
>> E; ++I)
>> +      if (CallInst *CI = dyn_cast<CallInst>(I))
>> +        if (CI->hasFnAttr(Attribute::Cold))
>> +          PostDominatedByColdCall.insert(BB);
>
> Here you should break out of the loop rather than continuing to scan 
> the basic
> block.

Good point.  Done.

> Otherwise LGTM.  Feel free to apply with the above breaking out of the 
> loop
> change.

Thanks.


Diego.



More information about the llvm-commits mailing list