[llvm] r348181 - [InstCombine] foldICmpWithLowBitMaskedVal(): disable 2 faulty folds.

[Roman Lebedev via llvm-commits]

On Tue, Dec 4, 2018 at 4:26 AM Friedman, Eli <efriedma at codeaurora.org> wrote:
>
> On 12/3/2018 12:07 PM, Roman Lebedev via llvm-commits wrote:
> > Author: lebedevri
> > Date: Mon Dec  3 12:07:58 2018
> > New Revision: 348181
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=348181&view=rev
> > Log:
> > [InstCombine] foldICmpWithLowBitMaskedVal(): disable 2 faulty folds.
> >
> > These two folds are invalid for this non-constant pattern
> > when the mask ends up being all-ones:
> > https://rise4fun.com/Alive/9au
> > https://rise4fun.com/Alive/UcQM
> >
> > Fixes https://bugs.llvm.org/show_bug.cgi?id=39861
> >
> > Modified:
> >      llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
> >      llvm/trunk/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sge-to-icmp-sle.ll
> >      llvm/trunk/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-slt-to-icmp-sgt.ll
> >
> > Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=348181&r1=348180&r2=348181&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
> > +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Mon Dec  3 12:07:58 2018
> > @@ -2955,12 +2955,16 @@ static Value *foldICmpWithLowBitMaskedVa
> >       //  x & (-1 >> y) s>= x    ->    x s<= (-1 >> y)
> >       if (X != I.getOperand(1)) // X must be on RHS of comparison!
> >         return nullptr;         // Ignore the other case.
> > +    if (!match(M, m_Constant())) // Can not do this fold with non-constant.
> > +      return nullptr;
>
> Is there some guarantee that the constant can't be -1?
No. It's an implicit assumption that such a trivial instsimplify fold
will have always already happened before we ever get here.
I should have at least added an assert here, i guess?

> I mean,
> obviously "and %x, -1" will eventually fold, but it's not clear it will
> fold before this code runs.
>
> -Eli
Roman.

> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>