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

[Chris Lattner]

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.

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