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

[Matthias Braun via llvm-commits]

MatzeB added a comment.

In http://reviews.llvm.org/D16204#327512, @reames wrote:

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


I added a counter and got 686810 Query instances for my testcase that runs for 1.5s not sure if that is enough to expect a 1-2% performance diff...


Repository:
  rL LLVM

http://reviews.llvm.org/D16204