[PATCH] D68930: [InstCombine] Shift amount reassociation in shifty sign bit test (PR43595)

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 13 11:32:21 PDT 2019


lebedev.ri created this revision.
lebedev.ri added reviewers: spatel, efriedma.
lebedev.ri added a project: LLVM.
Herald added a subscriber: hiraditya.

This problem consists of several parts:

- Basic sign bit extraction - `trunc? (?shr %x, (bitwidth(x)-1))`. This is trivial, and easy to do, we have a fold for it.
- Shift amount reassociation - if we have two identical shifts, and we can simplify-add their shift amounts together, then we likely can just perform them as a single shift. But this is finicky, has one-use restrictions, and shift opcodes must be identical.

But there is a super-pattern where both of these work together. to produce sign bit test from two shifts + comparison.
We do indeed already handle this in most cases. But since we get that fold transitively, it has one-use restrictions.
And what's worse, in this case the right-shifts aren't required to be identical, and we can't handle that transitively:

If the total shift amount is bitwidth-1, only a sign bit will remain in the output value.
But if we look at this from the perspective of two shifts, we can't fold - we can't possibly know what bit pattern
we'd produce via two shifts, it will be *some* kind of a mask produced from original sign bit, but we just can't
tell it's shape: https://rise4fun.com/Alive/cM0 https://rise4fun.com/Alive/9IN

But it will *only* contain sign bit and zeros. So from the perspective of sign bit test, we're good:
https://rise4fun.com/Alive/FRz https://rise4fun.com/Alive/qBU
Superb!

So the simplest solution is to extend `reassociateShiftAmtsOfTwoSameDirectionShifts()` to also have a
sudo-analysis mode that will ignore extra-uses, and will only check whether a) those are two right shifts
and b) they end up with bitwidth(x)-1 shift amount  and return either the original value that we sign-checking,
or null.

This does not have any functionality change for the existing `reassociateShiftAmtsOfTwoSameDirectionShifts()`.

https://bugs.llvm.org/show_bug.cgi?id=43595


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68930

Files:
  llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
  llvm/lib/Transforms/InstCombine/InstCombineInternal.h
  llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
  llvm/test/Transforms/InstCombine/sign-bit-test-via-right-shifting-all-other-bits.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68930.224788.patch
Type: text/x-patch
Size: 10058 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191013/3a3593ad/attachment.bin>


More information about the llvm-commits mailing list