[PATCH] D47891: [InstSimplify] shl nuw %x, %y -> %x iff KnownBits says %x is negative.

[Sanjay Patel via Phabricator via llvm-commits]

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