Add 'cold' attribute to functions

Duncan Sands baldrick at free.fr
Thu May 23 23:10:33 PDT 2013


Hi Diego,

>>>>> +  SmallVector<unsigned, 4> ColdEdges;
>>>>> +  SmallVector<unsigned, 4> NormalEdges;
>>>>
>>>> You could reserve TI->getNumSuccessors() space in these vectors. Just a
>>>> thought.
>>>
>>>
>>> I can't, really.  The number of successors is not a constexpr.
>>
>> ColdEdges.reserve(TI->getNumSuccessors());
>
> Ah, you meant dynamically.  I'm not familiar with SmallVector, is it really
> necessary to reserve instead of allowing the container to size itself
> automatically?  I've not seen that pattern elsewhere in this code.

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.

> +  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.

> +  }

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

Ciao, Duncan.



More information about the llvm-commits mailing list