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

[Roman Lebedev via llvm-commits]

On Tue, Dec 4, 2018 at 11:59 PM Friedman, Eli <efriedma at codeaurora.org> wrote:
>
> On 12/4/2018 5:21 AM, Roman Lebedev wrote:
> > 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?
>
> Not sure why you would expect that, in general, given the various ways
> we can call into InstSimplify.
InstSimplify - yes.
But this is in instcombine.
This is *not* run under -instsimplify.

> If there's some reason I'm missing that
> would make it impossible, please add an assert; otherwise please just
> add an early return.
>
> -Eli
Roman.

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