[PATCH] D11345: ignore duplicate divisor uses when transforming into reciprocal multiplies (PR24141)

Sanjay Patel spatel at rotateright.com
Mon Jul 20 20:25:04 PDT 2015


spatel added a comment.

In http://reviews.llvm.org/D11345#208610, @chandlerc wrote:

> In http://reviews.llvm.org/D11345#208493, @spatel wrote:
>
> > Patch updated:
> >  I just discovered 'SmallPtrSet'. 
> >  Looks like it was designed exactly for this case, so let's use it here instead of a vector. Then, we don't have to use std::find() for dup checking.
>
>
> Egads no, that makes the iteration order unstable.
>
> This whole thing seems wrongly formulated. Why are we walking the *entire* users list prior to finding out whether there are few enough users? I feel like the TLI interface should say how many can be handled rather than accepting a number we want to handle.
>
> And then the code here should be to insert into a SetVector until we exceed that threshold.


I'm not following. We're asking the TLI if we have exceeded a minimum threshold, not a maximum. If we exceed the threshold, we want to transform all divisions that have this divisor...so we always have to walk all users?


http://reviews.llvm.org/D11345







More information about the llvm-commits mailing list