[PATCH] D47891: [InstSimplify] shl nuw %x, %y -> %x iff KnownBits says %x is negative.
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 7 10:48:01 PDT 2018
spatel added a comment.
In https://reviews.llvm.org/D47891#1125275, @lebedev.ri wrote:
> In https://reviews.llvm.org/D47891#1125259, @spatel wrote:
>
> > I think we need to make a compile-time vs. perf win assessment on this one. This is probably true for any proposal that uses knownbits analysis in InstSimplify at this point. @efriedma said something that stuck with me in 1 of my proposals a while back: we could use knownbits on *every* value in instsimplify, but that would be very expensive and wouldn't gain much perf.
>
>
> It is of note that in `SimplifyRightShift()` (which is directly above this function), we always use `KnownBits` to check for the low bit.
That's a good point. I didn't see that.
>> So I'd like to see debug STATISTICs for this fold to see how often we have 'shl nuw' in real code and how often this fires. Test-suite or compiling clang/llvm itself should be reasonable benchmarks.
>
> To be more specific, you want the count of times we call that `computeKnownBits()`, and how many of those result in the fold, i.e.
>
> counter0++;
> if (computeKnownBits(Op0, Q.DL, /*Depth=*/0, Q.AC, Q.CxtI, Q.DT).isNegative()) {
> counter1++;
> return Op0;
> }
>
>
> ?
Yes. I suppose the proof of whether this is a good addition will be in the actual compile-time and perf results for test-suite, but I think it would be good to have this micro-stat for our knowledge. My guess is that this is very rare, and the fold is even rarer...but that's just a guess, so having the data will be good.
Repository:
rL LLVM
https://reviews.llvm.org/D47891
More information about the llvm-commits
mailing list