[llvm-commits] Review request: dominance frontier computation speedup

[Tobias Grosser]

On 11/28/2010 09:30 PM, Chris Lattner wrote:
>
> On Nov 28, 2010, at 5:50 PM, Cameron Zwarich wrote:
>
>> On Nov 28, 2010, at 3:04 PM, Chris Lattner wrote:
>>
>>> Jakob's right, SmallPtrSet was really intended to be a stack object.  Typically if you overflow a stack object, it is because you're in the "large" case, and you might as well go for one big allocation to avoid the reallocation.
>>>
>>> However, I guess it does make sense for "small" objects to be used in (already expensive) node-based containers like std::map.  I wouldn't be opposed to adding a template argument to control this, and have it get passed down as a bool to grow().
>>
>> Would you be opposed to adding a template parameter to SmallPtrSetImpl for this? It kind of sucks to have multiple compiled versions of the code, but the alternative would be passing a parameter into SmallPtrSetImpl::insert().
>
> Sure, go for it.  The SmallPtrSet templates are intended to be so trivial that they will always be inlined away.  All the meat should be out of line.
>
>>> Another concern: does anything depend on iteration over DF sets being in pointer order?  SmallPtrSet doesn't provide stable iteration.
>>
>> The IDF algorithm doesn't, and I am pretty sure that RegionInfo wouldn't need it either.
>
> Ok.  Ideally regioninfo will migrate off DomFrontier anyway.  In any case, nothing is using it.

I agree. This is on my medium term agenda. ;-)

>>> There are other low-hanging opportunities to speed up the existing dom frontiers implementation: the std::set in compareDomSet can trivially be converted to SmallPtrSet (and ideally moved out of line :) for example.
>>
>> As far as I can tell, compareDomSet is only used in the verification code. Is that worth speeding up?
>
> *shrug* if it happens in debug builds, it would be nice for them to go faster.  Not high priority though.
>
> -Chris

Tobi