[InstCombine] Fold select (icmp sgt x, -1), lshr (X, Y), ashr (X, Y) to ashr (X, Y))

[xbolva00]

Summary

(select (icmp sgt x, -1), lshr (X, Y), ashr (X, Y)) -> ashr (X, Y))
(select (icmp slt x, 1), ashr (X, Y), lshr (X, Y)) -> ashr (X, Y))

Fixes PR41173

Alive proof by @lebedev.ri (thanks)
Name: PR41173

%cmp = icmp slt i32 %x, 1
%shr = lshr i32 %x, %y
%shr1 = ashr i32 %x, %y
%retval.0 = select i1 %cmp, i32 %shr1, i32 %shr
=>
%retval.0 = ashr i32 %x, %y

Optimization: PR41173
Done: 1
Optimization is correct!

[xbolva00]

Updated tests

[spatel]

If this transform always returns an existing value, it can go in InstSimplify. Please pre-commit the baseline tests (in the InstSimplify directory and change the RUN line if I got that right).

[craig.topper]

What is one of the shifts has the "exact" flag set and the other doesn't?

[lebedev.ri]

Thanks for working on this!

In D64285#1572351, @spatel wrote:

If this transform always returns an existing value, it can go in InstSimplify. Please pre-commit the baseline tests (in the InstSimplify directory and change the RUN line if I got that right).

I agree.

In D64285#1572368, @craig.topper wrote:

What is one of the shifts has the "exact" flag set and the other doesn't?

We'd still drop one of two shifts, and a select, so this is really good.
But indeed. We can do this in InstSimplify only if both of the shifts either had or had no exact:
https://rise4fun.com/Alive/t4Z
https://rise4fun.com/Alive/P6a
https://rise4fun.com/Alive/gfP9

[xbolva00]

Moved to InstSimplify.
Precommited tests.
Handled "exact" variants.

[craig.topper]

Are we really allowed to change the exact flag from InstSimplify?

[lebedev.ri]

Thank you for working on this!

lib/Analysis/InstructionSimplify.cpp

82–85 ↗

(On Diff #208701)

There was some utility function to check for sign-bit tests, i think, i can't find it..

90–92 ↗

(On Diff #208701)

This modifies existing instruction.
I really don't think this is legal for instsimplify.

The mismatch case should stay in instcombine.

[spatel]

In D64285#1576143, @craig.topper wrote:

Are we really allowed to change the exact flag from InstSimplify?

No - we're not supposed to morph existing values here.

I wasn't thinking about the 'exact' case when I made the earlier comment, so we should move this back to instcombine. Sorry about that.

[xbolva00]

lib/Analysis/InstructionSimplify.cpp

82–85 ↗

(On Diff #208701)

Thanks for hint!

I found it, decomposeBitTestICmp.

[lebedev.ri]

lib/Analysis/InstructionSimplify.cpp

82–85 ↗

(On Diff #208701)

Yep, i think that's the one.

[xbolva00]

Moved back to InstCombine.

[xbolva00]

lib/Analysis/InstructionSimplify.cpp

82–85 ↗

(On Diff #208701)

Sadly, we cannot use it here. E.g.: slt x, 1

[lebedev.ri]

In D64285#1576611, @xbolva00 wrote:

Moved back to InstCombine.

I think we are going in circles here.
Part of this fold belongs to instsimplify: (either both are exact, or both non-exact, or ashr non-exact)
https://rise4fun.com/Alive/pleX
And whatever remains (only ashr is exact) should be folded to ashr i32 %x, %y in instcombine.
https://rise4fun.com/Alive/MSpl

lib/Transforms/InstCombine/InstCombineSelect.cpp

547–548 ↗

(On Diff #208754)

Actually, i don't think this is precise: https://rise4fun.com/Alive/Njpt

[xbolva00]

Part of this fold belongs to instsimplify:

almost same code on two places :/

[lebedev.ri]

In D64285#1576737, @xbolva00 wrote:

Part of this fold belongs to instsimplify:

almost same code on two places :/

Ugly, yeah, but powerful instsimplify is nice ^^

[lebedev.ri]

(no new comments, just marking as reviewed)

[xbolva00]

Updated, addressed review notes.

[lebedev.ri]

test/Transforms/InstCombine/ashr-lshr.ll

2–3 ↗

(On Diff #209178)

Precommit please :)

[nikic]

In D64285#1576787, @lebedev.ri wrote:

Ugly, yeah, but powerful instsimplify is nice ^^

I feel pretty strongly that this is the not the right tradeoff to make here. Unless there's some specific evidence that we benefit from having this in instsimplify, reducing code duplication is imho much more valuable than making sure that one out of thousands of ad-hoc folds runs in a few more cases than it otherwise would...

[lebedev.ri]

Nice, pretty much there!

lib/Analysis/InstructionSimplify.cpp

73–74 ↗

(On Diff #209178)

///   (select (icmp sgt x, C), lshr (X, Y), ashr (X, Y)); iff C s>= -1
///   (select (icmp slt x, C), ashr (X, Y), lshr (X, Y)); iff C s>= 0

77–78 ↗

(On Diff #209178)

I think it would be less confusing to reword as

/// iff either `ashr` is not `exact`, or `lshr` is 'exact' too.

86 ↗

(On Diff #209178)

/// Canonicalize so that `ashr` is in FalseVal.

92 ↗

(On Diff #209178)

/// If either the `ashr` is non-`exact`, or `lshr` is exact too, then we can just return the `ashr` instruction.

i think it may be less confusing to flip the if accordingly.

97–99 ↗

(On Diff #209178)

early return?

lib/Transforms/InstCombine/InstCombineSelect.cpp

539 ↗

(On Diff #209178)

I don't think we want to make any such restrictions.

/// into:
///   ashr (X, Y)
/// It is assumed that InstSimplify has already run.

547–565 ↗

(On Diff #209178)

Same comments as to instsimplify function - comments, early return

557–558 ↗

(On Diff #209178)

No need for this check.

559 ↗

(On Diff #209178)

I'd really recommend to create a new instruction.
It will result in same source line count, but is that much more transparent to the pass,
and is e.g. visible in -debug output.

[lebedev.ri]

In D64285#1580359, @nikic wrote:

In D64285#1576787, @lebedev.ri wrote:

Ugly, yeah, but powerful instsimplify is nice ^^

I feel pretty strongly that this is the not the right tradeoff to make here. Unless there's some specific evidence that we benefit from having this in instsimplify, reducing code duplication is imho much more valuable than making sure that one out of thousands of ad-hoc folds runs in a few more cases than it otherwise would...

Feel free to bring this up as RFC on llvm-dev.

[lebedev.ri]

lib/Transforms/InstCombine/InstCombineSelect.cpp

559 ↗

(On Diff #209178)

Err
+
/// This new ashr must not be exact.

[xbolva00]

Updated.

[xbolva00]

Fixed forgotten note.

[lebedev.ri]

lib/Analysis/InstructionSimplify.cpp

92–93 ↗

(On Diff #209178)

Hmm, also, as i've recently seen, cast<BinaryOperator> is going to break for constant expressions.
Let's just do the most obvious thing instead:

if (match(TrueVal, m_LShr(m_Value(X), m_Value(Y))) &&
    match(FalseVal, m_AShr(m_Specific(X), m_Specific(Y))) &&
    match(CmpLHS, m_Specific(X)) && 
    (!match(FalseVal, m_Exact()) || match(TrueVal, m_Exact())))

[lebedev.ri]

lib/Transforms/InstCombine/InstCombineSelect.cpp

562 ↗

(On Diff #209201)

There is no legality reason for this check.
We won't (should not) get here in any other case other than when we need to produce the exact-less ashr.

[xbolva00]

lib/Analysis/InstructionSimplify.cpp

92–93 ↗

(On Diff #209178)

m_Exact() not exists?

[xbolva00]

lib/Analysis/InstructionSimplify.cpp

92–93 ↗

(On Diff #209178)

m_Exact(m_BinOp()).. okay

[xbolva00]

Fixed notes.

[xbolva00]

Updated. Hopefully almost there.

[lebedev.ri]

It'd be really great to have at just least one positive non-splat-vector test for instsimplify and instcombine for each predicate (i.e. 4 more tests?)

lib/Analysis/InstructionSimplify.cpp

92–93 ↗

(On Diff #209178)

Ugh, right, sorry.
Though maybe m_Instruction(), not m_BinOp(); this really asks for m_Anything() matcher..

82–84 ↗

(On Diff #209204)

This is not going to work with non-splat vector constants, although it is trivial to make it,
i'd recommend to do this instead:

unsigned Bitwidth = CmpRHS->getType()->getScalarSizeInBits();
if ((Pred != ICmpInst::ICMP_SGT ||
     !match(CmpRHS,
            m_SpecificInt_ICMP(ICmpInst::ICMP_SGE, APInt(Bitwidth, -1)))) &&
    (Pred != ICmpInst::ICMP_SLT ||
     !match(CmpRHS,
            m_SpecificInt_ICMP(ICmpInst::ICMP_SGE, APInt(Bitwidth, 0))))) {

(m_SpecificInt_ICMP() is really fresh)

lib/Transforms/InstCombine/InstCombineSelect.cpp

548–553 ↗

(On Diff #209206)

Same remark about constant

561 ↗

(On Diff #209206)

Likewise, i think it's sufficient to cast to Instruction..

564 ↗

(On Diff #209206)

nice!

[xbolva00]

lib/Analysis/InstructionSimplify.cpp

92–93 ↗

(On Diff #209178)

m_Exact(m_Instruction()) - does not work too.

a) use m_Exact(m_Instruction(DummyInst))
b) m_Exact(m_BinOp())

?

[xbolva00]

Fixed some review notes.

[xbolva00]

Introduced m_Instruction().

[lebedev.ri]

In D64285#1580392, @lebedev.ri wrote:

In D64285#1580359, @nikic wrote:

In D64285#1576787, @lebedev.ri wrote:

Ugly, yeah, but powerful instsimplify is nice ^^

I feel pretty strongly that this is the not the right tradeoff to make here.

Unless there's some specific evidence that we benefit from having this in instsimplify,
reducing code duplication is imho much more valuable than making sure that
one out of thousands of ad-hoc folds runs in a few more cases than it otherwise would...

Feel free to bring this up as RFC on llvm-dev.

To elaborate, i don't disagree that this is truly a death of a thousand cuts,
*all* these folds are ad-hoc. Some will do wonders for some code patterns,
some will be useless drag for most of the code.

While theoretically we could teach the peephole pass all about
every possible pattern, it is infeasible in reality.
(at least until instcombine-SMT is here, will that be a glorious day!).

So what is feasible is teaching it about patterns that *were* observed
in real code. This is what i'm trying to do with my patches,
and i'd guess this may be the case with this patch, @xbolva00?

As to the question in the context of this patch, whether to only have the InstCombine part,
or duplicate it in InstSimplify thus not prohibiting other passes from being smarter,
is this a question of code duplication, or redundant matching?
The former can be workarounded with some refactoring to have a single function,
former is still the same death of a thousand cuts and can't be reasonably addressed
within existing framework.

lib/Analysis/InstructionSimplify.cpp

92–93 ↗

(On Diff #209178)

Blargh, sorry for leading this in circles.
Then let's go sideways and just do

if (match(TrueVal, m_LShr(m_Value(X), m_Value(Y))) &&
    match(FalseVal, m_AShr(m_Specific(X), m_Specific(Y))) &&
    match(CmpLHS, m_Specific(X)) &&
    (!cast<Instruction>(FalseVal)->isExact() ||
     cast<Instruction>(TrueVal)->isExact()))

as this appears to least mad solution *here*.

[xbolva00]

(yeah, I saw this missed optimization on the rust github issue tracker)

[nikic]

In D64285#1580922, @lebedev.ri wrote:

In D64285#1580392, @lebedev.ri wrote:

In D64285#1580359, @nikic wrote:

In D64285#1576787, @lebedev.ri wrote:

Ugly, yeah, but powerful instsimplify is nice ^^

I feel pretty strongly that this is the not the right tradeoff to make here.

Unless there's some specific evidence that we benefit from having this in instsimplify,
reducing code duplication is imho much more valuable than making sure that
one out of thousands of ad-hoc folds runs in a few more cases than it otherwise would...

Feel free to bring this up as RFC on llvm-dev.

To elaborate, i don't disagree that this is truly a death of a thousand cuts,
*all* these folds are ad-hoc. Some will do wonders for some code patterns,
some will be useless drag for most of the code.

While theoretically we could teach the peep-hole pass all about
every possible pattern, it is infeasible in reality.
(at least until instcombine-SMT is here, will that be a glorious day!).

So what is feasible is teaching it about patterns that *were* observed
in real code. This is what i'm trying to do with my patches,
and i'd guess this may be the case with this patch, @xbolva00?

As to the question in the context of this patch, whether to only have the InstCombine part,
or duplicate it in InstSimplify thus not prohibiting other passes from being smarter,
is this a question of code duplication, or redundant matching?
The former can be workarounded with some refactoring to have a single function,
former is still the same death of a thousand cuts and can't be reasonably addressed
within existing framework.

I think my comment came across more negative than intended. I think this is a useful patch (e.g. I remember seeing this pattern reported in https://github.com/rust-lang/rust/issues/58083). What I disagree with is the "premature compiler optimization" that is sometimes happening. The original motivation for this patch is very likely fully addressed by just having the InstCombine portion of this patch. But when new combines are implemented, it often happens that their scope is extended beyond what we know to be necessary, into more speculative territory.

Often these extensions are beneficial, for example because it is better to handle a more general pattern than some specific cases, or because a more general pattern naturally falls out of the matching mechanisms we use (typical example: splat vectors). But in some cases these extensions seem to happen more "because we can" than out of a reasonable expectation of utility.

In this specific case, yes, technically we could get an additional improvement by having both the instsimplify and instcombine folds, but it comes at the cost of both duplicate code and duplicate matching overhead. In such cases, a value judgement has to be made that may differ case-by-case. Is the additional complexity and compile-time cost worth the additional optimization opportunities? In this specific case, I don't think so, as it is rather unlikely the inststimplify fold will trigger in a context where the instcombine fold would not. But I understand that this is something two reasonable people may disagree on ;)

So to answer your question, it's about both code duplication and redundant matching. And while the death by a thousand cuts may be unavoidable, we should still try to not hasten it along unduly...

[nikic]

(I defer to your judgement though. If you think having the instsimplify variant is worthwhile, I definitely don't want to block this.)

[spatel]

In D64285#1580944, @nikic wrote:

So to answer your question, it's about both code duplication and redundant matching. And while the death by a thousand cuts may be unavoidable, we should still try to not hasten it along unduly...

I agree. Let's not add extra code for the 'exact' case if we don't need to. Ie, if we can do all of these transforms in instcombine alone by dropping the 'exact' flag, do that. Put another way: unless there's evidence that dropping the exact flag is going to cause a missed optimization, go with the 'less code is better' solution. We have precedence in instcombine/instsimplify for that trade-off.

[xbolva00]

Updated..

[lebedev.ri]

Sorry for so may triparounds :(
Please re-re-precommit the tests first.
I guess this will work, with 2 nits addressed :)

In D64285#1580958, @spatel wrote:

In D64285#1580944, @nikic wrote:

So to answer your question, it's about both code duplication and redundant matching. And while the death by a thousand cuts may be unavoidable, we should still try to not hasten it along unduly...

I agree. Let's not add extra code for the 'exact' case if we don't need to. Ie, if we can do all of these transforms in instcombine alone by dropping the 'exact' flag, do that. Put another way: unless there's evidence that dropping the exact flag is going to cause a missed optimization, go with the 'less code is better' solution. We have precedence in instcombine/instsimplify for that trade-off.

I think you were thinking of something else here, as this does not represent the problem at hand.
We fully can do this fold within instcombine, preserving exact when possible.
By having instsimplify fold we, well, enrich instsimplify utility.

lib/Transforms/InstCombine/InstCombineSelect.cpp

565–567 ↗

(On Diff #209294)

This is way more covoluted than it should be:

bool IsExact = Ashr->isExact() && cast<Instruction>(TrueVal)->isExact();

568 ↗

(On Diff #209294)

You already matched X and Y.

569 ↗

(On Diff #209294)

[optional] Do we really care about the name? I'd think we should take one from select then, not that it is accessible here.

[xbolva00]

Fixes

[xbolva00]

It should be fine now.

Thank you all for review notes!

[vitalybuka]

compiler now crashes on this file from CTMark https://reviews.llvm.org/F9549420

FAILED: CTMark/SPASS/CMakeFiles/SPASS.dir/list.c.o 
/usr/local/google/home/vitalybuka/src/llvm.git/out/ts_zn_true_Os_32/tools/timeit --summary CTMark/SPASS/CMakeFiles/SPASS.dir/list.c.o.time /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/bin/clang -DNDEBUG  -m32     -Os   -w -Werror=date-time -fno-strict-aliasing -DCLOCK_NO_TIMING -MD -MT CTMark/SPASS/CMakeFiles/SPASS.dir/list.c.o -MF CTMark/SPASS/CMakeFiles/SPASS.dir/list.c.o.d -o CTMark/SPASS/CMakeFiles/SPASS.dir/list.c.o   -c /usr/local/google/home/vitalybuka/src/llvm.git/test-suite/CTMark/SPASS/list.c
clang: ../../llvm-project/llvm/include/llvm/ADT/APInt.h:279: llvm::APInt::APInt(unsigned int, uint64_t, bool): Assertion `BitWidth && "bitwidth too small"' failed.
Stack dump:
0.	Program arguments: /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/bin/clang -cc1 -triple i386-unknown-linux-gnu -emit-obj -disable-free -main-file-name list.c -mrelocation-model static -mthread-model posix -relaxed-aliasing -fmath-errno -masm-verbose -mconstructor-aliases -fuse-init-array -target-cpu pentium4 -dwarf-column-info -debugger-tuning=gdb -coverage-notes-file /usr/local/google/home/vitalybuka/src/llvm.git/out/ts_zn_true_Os_32/CTMark/SPASS/CMakeFiles/SPASS.dir/list.c.gcno -resource-dir /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/lib/clang/9.0.0 -dependency-file CTMark/SPASS/CMakeFiles/SPASS.dir/list.c.o.d -sys-header-deps -MT CTMark/SPASS/CMakeFiles/SPASS.dir/list.c.o -D NDEBUG -D CLOCK_NO_TIMING -internal-isystem /usr/local/include -internal-isystem /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/lib/clang/9.0.0/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -Os -Werror=date-time -w -fdebug-compilation-dir /usr/local/google/home/vitalybuka/src/llvm.git/out/ts_zn_true_Os_32 -ferror-limit 19 -fmessage-length 0 -fobjc-runtime=gcc -fdiagnostics-show-option -vectorize-loops -vectorize-slp -faddrsig -o CTMark/SPASS/CMakeFiles/SPASS.dir/list.c.o -x c /usr/local/google/home/vitalybuka/src/llvm.git/test-suite/CTMark/SPASS/list.c 
1.	<eof> parser at end of file
2.	Per-module optimization passes
3.	Running pass 'CallGraph Pass Manager' on module '/usr/local/google/home/vitalybuka/src/llvm.git/test-suite/CTMark/SPASS/list.c'.
4.	Running pass 'Combine redundant instructions' on function '@list_CompareElementDistribution'
 #0 0x000000000296d71d llvm::sys::PrintStackTrace(llvm::raw_ostream&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Support/Unix/Signals.inc:494:13
 #1 0x000000000296d71d PrintStackTraceSignalHandler(void*) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Support/Unix/Signals.inc:554:0
 #2 0x000000000296b3fe llvm::sys::RunSignalHandlers() /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Support/Signals.cpp:69:18
 #3 0x000000000296d8d8 SignalHandler(int) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Support/Unix/Signals.inc:357:1
 #4 0x00007f342ae543a0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x123a0)
 #5 0x00007f3429f7ecfb raise (/lib/x86_64-linux-gnu/libc.so.6+0x36cfb)
 #6 0x00007f3429f698ad abort (/lib/x86_64-linux-gnu/libc.so.6+0x218ad)
 #7 0x00007f3429f6977f (/lib/x86_64-linux-gnu/libc.so.6+0x2177f)
 #8 0x00007f3429f77542 (/lib/x86_64-linux-gnu/libc.so.6+0x2f542)
 #9 0x0000000002b401e1 llvm::InstCombiner::foldSelectInstWithICmp(llvm::SelectInst&, llvm::ICmpInst*) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:0:0
#10 0x0000000002b41b87 llvm::InstCombiner::visitSelectInst(llvm::SelectInst&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:1963:31
#11 0x0000000002ab13d2 llvm::InstCombiner::run() /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3289:22
#12 0x0000000002ab2d6a combineInstructionsOverFunction(llvm::Function&, llvm::InstCombineWorklist&, llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, llvm::DominatorTree&, llvm::OptimizationRemarkEmitter&, llvm::BlockFrequencyInfo*, llvm::ProfileSummaryInfo*, bool, llvm::LoopInfo*) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3523:13
#13 0x0000000002ab4338 llvm::InstructionCombiningPass::runOnFunction(llvm::Function&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3597:10
#14 0x00000000027a642f llvm::FPPassManager::runOnFunction(llvm::Function&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1648:27
#15 0x00000000024bc7e7 (anonymous namespace)::CGPassManager::RunPassOnSCC(llvm::Pass*, llvm::CallGraphSCC&, llvm::CallGraph&, bool&, bool&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Analysis/CallGraphSCCPass.cpp:176:25
#16 0x00000000024bc7e7 (anonymous namespace)::CGPassManager::RunAllPassesOnSCC(llvm::CallGraphSCC&, llvm::CallGraph&, bool&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Analysis/CallGraphSCCPass.cpp:441:0
#17 0x00000000024bc7e7 (anonymous namespace)::CGPassManager::runOnModule(llvm::Module&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/Analysis/CallGraphSCCPass.cpp:497:0
#18 0x00000000027a6e56 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1752:27
#19 0x00000000027a6e56 llvm::legacy::PassManagerImpl::run(llvm::Module&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1865:0
#20 0x0000000001b06e4b (anonymous namespace)::EmitAssemblyHelper::EmitAssembly(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/clang/lib/CodeGen/BackendUtil.cpp:902:3
#21 0x0000000001b06e4b clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayout const&, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/clang/lib/CodeGen/BackendUtil.cpp:1549:0
#22 0x0000000002106e5d std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >::~unique_ptr() /usr/lib/gcc/x86_64-linux-gnu/8.0.1/../../../../include/c++/8.0.1/bits/unique_ptr.h:273:6
#23 0x0000000002106e5d clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/clang/lib/CodeGen/CodeGenAction.cpp:303:0
#24 0x0000000003417a75 __gnu_cxx::__normal_iterator<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >*, std::vector<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >, std::allocator<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> > > > >::__normal_iterator(std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >* const&) /usr/lib/gcc/x86_64-linux-gnu/8.0.1/../../../../include/c++/8.0.1/bits/stl_iterator.h:781:20
#25 0x0000000003417a75 std::vector<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >, std::allocator<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> > > >::begin() /usr/lib/gcc/x86_64-linux-gnu/8.0.1/../../../../include/c++/8.0.1/bits/stl_vector.h:699:0
#26 0x0000000003417a75 void clang::finalize<std::vector<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >, std::allocator<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> > > > >(std::vector<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >, std::allocator<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> > > >&, clang::Sema const&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/clang/include/clang/Sema/TemplateInstCallback.h:54:0
#27 0x0000000003417a75 clang::ParseAST(clang::Sema&, bool, bool) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/clang/lib/Parse/ParseAST.cpp:178:0
#28 0x0000000002065250 clang::FrontendAction::Execute() /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/clang/lib/Frontend/FrontendAction.cpp:938:10
#29 0x0000000001fcf871 llvm::Error::getPtr() const /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/include/llvm/Support/Error.h:273:42
#30 0x0000000001fcf871 llvm::Error::operator bool() /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/llvm/include/llvm/Support/Error.h:236:0
#31 0x0000000001fcf871 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/clang/lib/Frontend/CompilerInstance.cpp:944:0
#32 0x000000000210205f clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:291:25
#33 0x0000000001ac0975 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/clang/tools/driver/cc1_main.cpp:249:15
#34 0x0000000001ace43b ExecuteCC1Tool(llvm::ArrayRef<char const*>, llvm::StringRef) /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/clang/tools/driver/driver.cpp:309:12
#35 0x0000000001ace43b main /usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/../../llvm-project/clang/tools/driver/driver.cpp:381:0
#36 0x00007f3429f6b52b __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2352b)
#37 0x0000000001ac002a _start (/usr/local/google/home/vitalybuka/src/llvm.git/out/zn_true/bin/clang+0x1ac002a)

[lebedev.ri]

llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp

547

So this returns 0 i guess?
That is confusing..

[xbolva00]

llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp

547

Yeah, weird.

@spatel ?

[craig.topper]

getScalarSizeInBits probably returns 0 for pointers. Only DataLayout knows pointer size.

[xbolva00]

if (Bitwidth == 0) {

  IC->dump();
}

%cmp.i = icmp ne %struct.LIST_HELP* %List.addr.0, null

[xbolva00]

In D64285#1583581, @craig.topper wrote:

getScalarSizeInBits probably returns 0 for pointers. Only DataLayout knows pointer size.

Thanks

[craig.topper]

In D64285#1583589, @xbolva00 wrote:

if (Bitwidth == 0) {

  IC->dump();
}

%cmp.i = icmp ne %struct.LIST_HELP* %List.addr.0, null

Yep, its a pointer. Just disable the transform if !CmpRHs->getType()->isIntOrIntVectorTy()

[xbolva00]

In D64285#1583596, @craig.topper wrote:

In D64285#1583589, @xbolva00 wrote:

if (Bitwidth == 0) {

  IC->dump();
}

%cmp.i = icmp ne %struct.LIST_HELP* %List.addr.0, null

Yep, its a pointer. Just disable the transform if !CmpRHs->getType()->isIntOrIntVectorTy()

Thanks! Commited fast fix. And thanks @vitalybuka for reporting this issue.

rL365959

llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp

static Instruction *foldSelectICmpAndAnd(Type *SelType, const ICmpInst *Cmp,
   Value *MaskB = HasShift ? Builder.CreateShl(One, Z) : One;
   Value *FullMask = Builder.CreateOr(Y, MaskB);
   Value *MaskedX = Builder.CreateAnd(X, FullMask);
   Value *ICmpNeZero = Builder.CreateIsNotNull(MaskedX);
   return new ZExtInst(ICmpNeZero, SelType);
 }
 
 /// We want to turn:
+///   (select (icmp sgt x, C), lshr (X, Y), ashr (X, Y)); iff C s>= -1
+///   (select (icmp slt x, C), ashr (X, Y), lshr (X, Y)); iff C s>= 0
+/// into:
+///   ashr (X, Y)
+static Value *foldSelectICmpLshrAshr(const ICmpInst *IC, Value *TrueVal,
+                                     Value *FalseVal,
+                                     InstCombiner::BuilderTy &Builder) {
+  ICmpInst::Predicate Pred = IC->getPredicate();
+  Value *CmpLHS = IC->getOperand(0);
+  Value *CmpRHS = IC->getOperand(1);
+
+  Value *X, *Y;
+  unsigned Bitwidth = CmpRHS->getType()->getScalarSizeInBits();
+  if ((Pred != ICmpInst::ICMP_SGT ||
+       !match(CmpRHS,
+              m_SpecificInt_ICMP(ICmpInst::ICMP_SGE, APInt(Bitwidth, -1)))) &&
+      (Pred != ICmpInst::ICMP_SLT ||
+       !match(CmpRHS,
+              m_SpecificInt_ICMP(ICmpInst::ICMP_SGE, APInt(Bitwidth, 0)))))
+    return nullptr;
+
+  // Canonicalize so that ashr is in FalseVal.
+  if (Pred == ICmpInst::ICMP_SLT)
+    std::swap(TrueVal, FalseVal);
+
+  if (match(TrueVal, m_LShr(m_Value(X), m_Value(Y))) &&
+      match(FalseVal, m_AShr(m_Specific(X), m_Specific(Y))) &&
+      match(CmpLHS, m_Specific(X))) {
+    const auto *Ashr = cast<Instruction>(FalseVal);
+    // if lshr is not exact and ashr is, this new ashr must not be exact.
+    bool IsExact = Ashr->isExact() && cast<Instruction>(TrueVal)->isExact();
+    return Builder.CreateAShr(X, Y, IC->getName(), IsExact);
+  }
+
+  return nullptr;
+}
+
+/// We want to turn:
 ///   (select (icmp eq (and X, C1), 0), Y, (or Y, C2))
 /// into:
 ///   (or (shl (and X, C1), C3), Y)
 /// iff:
 ///   C1 and C2 are both powers of 2
 /// where:
 ///   C3 = Log(C2) - Log(C1)
 ///
Instruction *InstCombiner::foldSelectInstWithICmp(SelectInst &SI,
 
   if (Instruction *V =
           foldSelectICmpAndAnd(SI.getType(), ICI, TrueVal, FalseVal, Builder))
     return V;
 
   if (Value *V = foldSelectICmpAndOr(ICI, TrueVal, FalseVal, Builder))
     return replaceInstUsesWith(SI, V);
 
+  if (Value *V = foldSelectICmpLshrAshr(ICI, TrueVal, FalseVal, Builder))
+    return replaceInstUsesWith(SI, V);
+
   if (Value *V = foldSelectCttzCtlz(ICI, TrueVal, FalseVal, Builder))
     return replaceInstUsesWith(SI, V);
 
   if (Value *V = canonicalizeSaturatedSubtract(ICI, TrueVal, FalseVal, Builder))
     return replaceInstUsesWith(SI, V);
 
   if (Value *V = canonicalizeSaturatedAdd(ICI, TrueVal, FalseVal, Builder))
     return replaceInstUsesWith(SI, V);

llvm/trunk/test/Transforms/InstCombine/ashr-lshr.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
 define i32 @ashr_lshr_exact_ashr_only(i32 %x, i32 %y) {
 ; CHECK-LABEL: @ashr_lshr_exact_ashr_only(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[X:%.*]], -1
-; CHECK-NEXT:    [[L:%.*]] = lshr i32 [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact i32 [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select i1 [[CMP]], i32 [[L]], i32 [[R]]
-; CHECK-NEXT:    ret i32 [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i32 [[CMP1]]
 ;
   %cmp = icmp sgt i32 %x, -1
   %l = lshr i32 %x, %y
   %r = ashr exact i32 %x, %y
   %ret = select i1 %cmp, i32 %l, i32 %r
   ret i32 %ret
 }
 
 define i32 @ashr_lshr_no_exact(i32 %x, i32 %y) {
 ; CHECK-LABEL: @ashr_lshr_no_exact(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[X:%.*]], -1
-; CHECK-NEXT:    [[L:%.*]] = lshr i32 [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr i32 [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select i1 [[CMP]], i32 [[L]], i32 [[R]]
-; CHECK-NEXT:    ret i32 [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i32 [[CMP1]]
 ;
   %cmp = icmp sgt i32 %x, -1
   %l = lshr i32 %x, %y
   %r = ashr i32 %x, %y
   %ret = select i1 %cmp, i32 %l, i32 %r
   ret i32 %ret
 }
 
 define i32 @ashr_lshr_exact_both(i32 %x, i32 %y) {
 ; CHECK-LABEL: @ashr_lshr_exact_both(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[X:%.*]], -1
-; CHECK-NEXT:    [[L:%.*]] = lshr exact i32 [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact i32 [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select i1 [[CMP]], i32 [[L]], i32 [[R]]
-; CHECK-NEXT:    ret i32 [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr exact i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i32 [[CMP1]]
 ;
   %cmp = icmp sgt i32 %x, -1
   %l = lshr exact i32 %x, %y
   %r = ashr exact i32 %x, %y
   %ret = select i1 %cmp, i32 %l, i32 %r
   ret i32 %ret
 }
 
 define i32 @ashr_lshr_exact_lshr_only(i32 %x, i32 %y) {
 ; CHECK-LABEL: @ashr_lshr_exact_lshr_only(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[X:%.*]], -1
-; CHECK-NEXT:    [[L:%.*]] = lshr exact i32 [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr i32 [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select i1 [[CMP]], i32 [[L]], i32 [[R]]
-; CHECK-NEXT:    ret i32 [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i32 [[CMP1]]
 ;
   %cmp = icmp sgt i32 %x, -1
   %l = lshr exact i32 %x, %y
   %r = ashr i32 %x, %y
   %ret = select i1 %cmp, i32 %l, i32 %r
   ret i32 %ret
 }
 
 define i32 @ashr_lshr2(i32 %x, i32 %y) {
 ; CHECK-LABEL: @ashr_lshr2(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[X:%.*]], 5
-; CHECK-NEXT:    [[L:%.*]] = lshr i32 [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact i32 [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select i1 [[CMP]], i32 [[L]], i32 [[R]]
-; CHECK-NEXT:    ret i32 [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i32 [[CMP1]]
 ;
   %cmp = icmp sgt i32 %x, 5
   %l = lshr i32 %x, %y
   %r = ashr exact i32 %x, %y
   %ret = select i1 %cmp, i32 %l, i32 %r
   ret i32 %ret
 }
 
 define <2 x i32> @ashr_lshr_splat_vec(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_splat_vec(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt <2 x i32> [[X:%.*]], <i32 -1, i32 -1>
-; CHECK-NEXT:    [[L:%.*]] = lshr <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[L]], <2 x i32> [[R]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp sgt <2 x i32> %x, <i32 -1, i32 -1>
   %l = lshr <2 x i32> %x, %y
   %r = ashr <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %l, <2 x i32> %r
   ret <2 x i32> %ret
 }
 
 define <2 x i32> @ashr_lshr_splat_vec2(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_splat_vec2(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt <2 x i32> [[X:%.*]], <i32 -1, i32 -1>
-; CHECK-NEXT:    [[L:%.*]] = lshr exact <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[L]], <2 x i32> [[R]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr exact <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp sgt <2 x i32> %x, <i32 -1, i32 -1>
   %l = lshr exact <2 x i32> %x, %y
   %r = ashr exact <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %l, <2 x i32> %r
   ret <2 x i32> %ret
 }
 
 define <2 x i32> @ashr_lshr_splat_vec3(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_splat_vec3(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt <2 x i32> [[X:%.*]], <i32 -1, i32 -1>
-; CHECK-NEXT:    [[L:%.*]] = lshr exact <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[L]], <2 x i32> [[R]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp sgt <2 x i32> %x, <i32 -1, i32 -1>
   %l = lshr exact <2 x i32> %x, %y
   %r = ashr <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %l, <2 x i32> %r
   ret <2 x i32> %ret
 }
 
 define <2 x i32> @ashr_lshr_splat_vec4(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_splat_vec4(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt <2 x i32> [[X:%.*]], <i32 -1, i32 -1>
-; CHECK-NEXT:    [[L:%.*]] = lshr <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[L]], <2 x i32> [[R]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp sgt <2 x i32> %x, <i32 -1, i32 -1>
   %l = lshr <2 x i32> %x, %y
   %r = ashr exact <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %l, <2 x i32> %r
   ret <2 x i32> %ret
 }
 
 define <2 x i32> @ashr_lshr_nonsplat_vec(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_nonsplat_vec(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt <2 x i32> [[X:%.*]], <i32 -1, i32 1>
-; CHECK-NEXT:    [[L:%.*]] = lshr <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[L]], <2 x i32> [[R]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp sgt <2 x i32> %x, <i32 -1, i32 1>
   %l = lshr <2 x i32> %x, %y
   %r = ashr <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %l, <2 x i32> %r
   ret <2 x i32> %ret
 }
 
 define <2 x i32> @ashr_lshr_nonsplat_vec2(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_nonsplat_vec2(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt <2 x i32> [[X:%.*]], <i32 2, i32 4>
-; CHECK-NEXT:    [[L:%.*]] = lshr exact <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[L]], <2 x i32> [[R]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr exact <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp sgt <2 x i32> %x, <i32 2, i32 4>
   %l = lshr exact <2 x i32> %x, %y
   %r = ashr exact <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %l, <2 x i32> %r
   ret <2 x i32> %ret
 }
 
 define <2 x i32> @ashr_lshr_nonsplat_vec3(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_nonsplat_vec3(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt <2 x i32> [[X:%.*]], <i32 5, i32 6>
-; CHECK-NEXT:    [[L:%.*]] = lshr exact <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[L]], <2 x i32> [[R]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp sgt <2 x i32> %x, <i32 5, i32 6>
   %l = lshr exact <2 x i32> %x, %y
   %r = ashr <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %l, <2 x i32> %r
   ret <2 x i32> %ret
 }
 
 define <2 x i32> @ashr_lshr_nonsplat_vec4(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_nonsplat_vec4(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt <2 x i32> [[X:%.*]], <i32 8, i32 7>
-; CHECK-NEXT:    [[L:%.*]] = lshr <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[L]], <2 x i32> [[R]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp sgt <2 x i32> %x, <i32 8, i32 7>
   %l = lshr <2 x i32> %x, %y
   %r = ashr exact <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %l, <2 x i32> %r
   ret <2 x i32> %ret
 }
 
 define i32 @ashr_lshr_cst(i32 %x, i32 %y) {
 ; CHECK-LABEL: @ashr_lshr_cst(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[X:%.*]], 1
-; CHECK-NEXT:    [[L:%.*]] = lshr i32 [[X]], 8
-; CHECK-NEXT:    [[R:%.*]] = ashr exact i32 [[X]], 8
-; CHECK-NEXT:    [[RET:%.*]] = select i1 [[CMP]], i32 [[R]], i32 [[L]]
-; CHECK-NEXT:    ret i32 [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr i32 [[X:%.*]], 8
+; CHECK-NEXT:    ret i32 [[CMP1]]
 ;
   %cmp = icmp slt i32 %x, 1
   %l = lshr i32 %x, 8
   %r = ashr exact i32 %x, 8
   %ret = select i1 %cmp, i32 %r, i32 %l
   ret i32 %ret
 }
 
 define i32 @ashr_lshr_cst2(i32 %x, i32 %y) {
 ; CHECK-LABEL: @ashr_lshr_cst2(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[X:%.*]], -1
-; CHECK-NEXT:    [[L:%.*]] = lshr i32 [[X]], 8
-; CHECK-NEXT:    [[R:%.*]] = ashr exact i32 [[X]], 8
-; CHECK-NEXT:    [[RET:%.*]] = select i1 [[CMP]], i32 [[L]], i32 [[R]]
-; CHECK-NEXT:    ret i32 [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr i32 [[X:%.*]], 8
+; CHECK-NEXT:    ret i32 [[CMP1]]
 ;
   %cmp = icmp sgt i32 %x, -1
   %l = lshr i32 %x, 8
   %r = ashr exact i32 %x, 8
   %ret = select i1 %cmp, i32 %l, i32 %r
   ret i32 %ret
 }
 
 define i32 @ashr_lshr_inv(i32 %x, i32 %y) {
 ; CHECK-LABEL: @ashr_lshr_inv(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[X:%.*]], 1
-; CHECK-NEXT:    [[L:%.*]] = lshr i32 [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact i32 [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select i1 [[CMP]], i32 [[R]], i32 [[L]]
-; CHECK-NEXT:    ret i32 [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i32 [[CMP1]]
 ;
   %cmp = icmp slt i32 %x, 1
   %l = lshr i32 %x, %y
   %r = ashr exact i32 %x, %y
   %ret = select i1 %cmp, i32 %r, i32 %l
   ret i32 %ret
 }
 
 define i32 @ashr_lshr_inv2(i32 %x, i32 %y) {
 ; CHECK-LABEL: @ashr_lshr_inv2(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[X:%.*]], 7
-; CHECK-NEXT:    [[L:%.*]] = lshr i32 [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact i32 [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select i1 [[CMP]], i32 [[R]], i32 [[L]]
-; CHECK-NEXT:    ret i32 [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i32 [[CMP1]]
 ;
   %cmp = icmp slt i32 %x, 7
   %l = lshr i32 %x, %y
   %r = ashr exact i32 %x, %y
   %ret = select i1 %cmp, i32 %r, i32 %l
   ret i32 %ret
 }
 
 define <2 x i32> @ashr_lshr_inv_splat_vec(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_inv_splat_vec(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp slt <2 x i32> [[X:%.*]], <i32 1, i32 1>
-; CHECK-NEXT:    [[L:%.*]] = lshr <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[R]], <2 x i32> [[L]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp slt <2 x i32> %x, <i32 1, i32 1>
   %l = lshr <2 x i32> %x, %y
   %r = ashr exact <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %r, <2 x i32> %l
   ret <2 x i32> %ret
 }
 
 define <2 x i32> @ashr_lshr_inv_nonsplat_vec(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_inv_nonsplat_vec(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp slt <2 x i32> [[X:%.*]], <i32 4, i32 5>
-; CHECK-NEXT:    [[L:%.*]] = lshr <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[R]], <2 x i32> [[L]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp slt <2 x i32> %x, <i32 4, i32 5>
   %l = lshr <2 x i32> %x, %y
   %r = ashr exact <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %r, <2 x i32> %l
   ret <2 x i32> %ret
 }
 
 define <2 x i32> @ashr_lshr_vec_undef(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_vec_undef(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt <2 x i32> [[X:%.*]], <i32 undef, i32 -1>
-; CHECK-NEXT:    [[L:%.*]] = lshr <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[L]], <2 x i32> [[R]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp sgt <2 x i32> %x, <i32 undef, i32 -1>
   %l = lshr <2 x i32> %x, %y
   %r = ashr exact <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %l, <2 x i32> %r
   ret <2 x i32> %ret
 }
 
 define <2 x i32> @ashr_lshr_vec_undef2(<2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @ashr_lshr_vec_undef2(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp slt <2 x i32> [[X:%.*]], <i32 1, i32 undef>
-; CHECK-NEXT:    [[L:%.*]] = lshr exact <2 x i32> [[X]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ashr exact <2 x i32> [[X]], [[Y]]
-; CHECK-NEXT:    [[RET:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[R]], <2 x i32> [[L]]
-; CHECK-NEXT:    ret <2 x i32> [[RET]]
+; CHECK-NEXT:    [[CMP1:%.*]] = ashr exact <2 x i32> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[CMP1]]
 ;
   %cmp = icmp slt <2 x i32> %x, <i32 1, i32 undef>
   %l = lshr exact <2 x i32> %x, %y
   %r = ashr exact <2 x i32> %x, %y
   %ret = select <2 x i1> %cmp, <2 x i32> %r, <2 x i32> %l
   ret <2 x i32> %ret
 }