[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