[all-commits] [llvm/llvm-project] 49483a: [InstCombine] Shift amount reassociation in shifty...

Roman Lebedev via All-commits all-commits at lists.llvm.org
Sun Oct 20 12:37:32 PDT 2019


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 49483a3bc2253c9e252e5e37b709534e3b6e51cc
      https://github.com/llvm/llvm-project/commit/49483a3bc2253c9e252e5e37b709534e3b6e51cc
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2019-10-20 (Sun, 20 Oct 2019)

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

  Log Message:
  -----------
  [InstCombine] Shift amount reassociation in shifty sign bit test (PR43595)

Summary:
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()`.

All that being said, as disscussed in the review, this yet again
increases usage of instsimplify in instcombine as utility.
Some day that may need to be reevaluated.

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

Reviewers: spatel, efriedma, vsk

Reviewed By: spatel

Subscribers: xbolva00, hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D68930

llvm-svn: 375371




More information about the All-commits mailing list