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