[PATCH] D16204: ValueTracking: Use fixed array for assumption exclude set in Query; NFC

[Philip Reames via llvm-commits]

reames added a comment.

In http://reviews.llvm.org/D16204#327492, @MatzeB wrote:

> In http://reviews.llvm.org/D16204#327491, @reames wrote:
>
> > Given MaxDepth is less than the Small size specified for the SmallPtrSet, I'm not sure why this is faster?  The only difference I can come up with in the new code vs the old is a) the check for isSmall and b) the fact the insert code is outlined for SmallPtrSet.
> >
> > I'd really doubt that (a) is the cause.  If it is (b), we should just fix the SmallPtrSet impl to inline the common case of the insertion logic.
> >
> > Please try measuring with a tweaked version of SmallPtrSet.  Unless you can show that the hand rolled code is still much faster, I'm hesitant to take this change.
>
>
> In the majority of the cases the set isn't used at all, I didn't see any performance differences when tweaking the insertion/query logic.


Wow, that was a fast turn around.  How'd you managed to build something after touching an ADT header that fast much less performance test it?  I want to know your secret.  :)

I have to admit, this really surprises me.

> The gains appear to come from the simplified initialization (and maybe reduced stack usage?).


Can you back up this claim?  I'm open to being convinced this is true, but it would imply that we're creating *a lot* of Query objects.  Do you believe that to be true?  If so, we could explore using a stack discipline for the exclusions rather than passing around copies of the full Query.


Repository:
  rL LLVM

http://reviews.llvm.org/D16204