[llvm-dev] Hitting assertion failure related to vectorization + instcombine

Thu Jul 28 08:32:59 PDT 2016

Back-ported to 3.9 in r276986.

Thanks,
Hans

On Wed, Jul 27, 2016 at 5:19 PM, David Majnemer
<david.majnemer at gmail.com> wrote:
> LGTM
>
>
> On Wednesday, July 27, 2016, Hans Wennborg <hans at chromium.org> wrote:
>>
>> David, Sanjay: ping?
>>
>> On Mon, Jul 25, 2016 at 11:07 AM, Hans Wennborg <hans at chromium.org> wrote:
>> > Sure. David, what do you think about merging this to 3.9?
>> >
>> > Sanjay: are you saying I'd just apply that diff to
>> > InstructionSimplify.cpp, not InstCombineSelect.cpp?
>> >
>> > On Fri, Jul 22, 2016 at 7:08 AM, Sanjay Patel <spatel at rotateright.com>
>> > wrote:
>> >> Hi Hans -
>> >>
>> >> Yes, I think this is a good patch for 3.9 (cc'ing David Majnemer as
>> >> code
>> >> owner). The functional change was r276209. I made an NFC refactoring in
>> >> InstSimplify at r275911 that isn't in the branch, so I don't think the
>> >> patch
>> >> will apply as-is. For safety, you could apply the one-line fix without
>> >> pulling the refactoring into the branch with this diff:
>> >>
>> >> -    unsigned BitWidth = Q.DL.getTypeSizeInBits(TrueVal->getType());
>> >> +    unsigned BitWidth =
>> >> +        Q.DL.getTypeSizeInBits(TrueVal->getType()->getScalarType());
>> >>
>> >>
>> >>
>> >>
>> >> On Fri, Jul 22, 2016 at 7:45 AM, Hans Wennborg <hans at chromium.org>
>> >> wrote:
>> >>>
>> >>> Sanjay: let me know if this is something that will apply to 3.9.
>> >>>
>> >>> Thanks,
>> >>> Hans
>> >>>
>> >>> On Wed, Jul 20, 2016 at 5:59 PM, Sanjay Patel via llvm-dev
>> >>> <llvm-dev at lists.llvm.org> wrote:
>> >>> > Quick update - the bug existed before I refactored that chunk in
>> >>> > InstSimplify with:
>> >>> > https://reviews.llvm.org/rL275911
>> >>> >
>> >>> > In fact, as discussed in https://reviews.llvm.org/D22537 - because
>> >>> > we
>> >>> > have a
>> >>> > big pile of lazily copied code, the bug has an identical twin that
>> >>> > exists in
>> >>> > InstCombine. I swear I didn't touch that code...yet. :)
>> >>> >
>> >>> > define <2 x i32> @select_icmp_vec(<2 x i32> %x) {
>> >>> >   %cmp = icmp slt <2 x i32> %x, zeroinitializer
>> >>> >   %xor = xor <2 x i32> %x, <i32 2147483648, i32 2147483648>
>> >>> >   %x.xor = select <2 x i1> %cmp, <2 x i32> %x, <2 x i32> %xor
>> >>> >   ret <2 x i32> %x.xor
>> >>> > }
>> >>> >
>> >>> > $ ./opt -instcombine selvec.ll -S
>> >>> > Assertion failed: (BitWidth == RHS.BitWidth && "Comparison requires
>> >>> > equal
>> >>> > bit widths"), function operator==, file
>> >>> > /Users/spatel/myllvm/llvm/include/llvm/ADT/APInt.h, line 983.
>> >>> >
>> >>> > I should have a patch up for review shortly.
>> >>> >
>> >>> >
>> >>> > On Wed, Jul 20, 2016 at 2:03 PM, Sanjay Patel
>> >>> > <spatel at rotateright.com>
>> >>> > wrote:
>> >>> >>
>> >>> >> Thanks for notifying me. Yes, this was a recent change. Taking a
>> >>> >> look
>> >>> >> now.
>> >>> >>
>> >>> >> On Wed, Jul 20, 2016 at 1:49 PM, Michael Kuperstein
>> >>> >> <mkuper at google.com>
>> >>> >> wrote:
>> >>> >>>
>> >>> >>> +Sanjay, who touched this last. :-)
>> >>> >>>
>> >>> >>> On Wed, Jul 20, 2016 at 12:44 PM, Ismail Badawi (ibadawi) via
>> >>> >>> llvm-dev
>> >>> >>> <llvm-dev at lists.llvm.org> wrote:
>> >>> >>>>
>> >>> >>>> Hi folks,
>> >>> >>>>
>> >>> >>>> I'm hitting the below assertion failure when compiling this small
>> >>> >>>> piece
>> >>> >>>> of C code (repro.c, attached).
>> >>> >>>>
>> >>> >>>> My command line is:
>> >>> >>>>
>> >>> >>>> bin/clang --target=aarch64-linux-gnu -c -O2 repro.c
>> >>> >>>>
>> >>> >>>> clang is built from top of trunk as of this morning. It only
>> >>> >>>> happens
>> >>> >>>> at
>> >>> >>>> -O2, and it doesn't happen with the default target (x86_64). I
>> >>> >>>> tried
>> >>> >>>> to
>> >>> >>>> reproduce using just 'llc -O2' but didn't manage -- but I do have
>> >>> >>>> this
>> >>> >>>> reduced opt command line (repro.ll also attached, just generated
>> >>> >>>> from
>> >>> >>>> repro.c at -O0):
>> >>> >>>>
>> >>> >>>> bin/opt -instcombine -licm -simplifycfg -instcombine -loop-rotate
>> >>> >>>> -loop-vectorize -instcombine < repro.ll
>> >>> >>>>
>> >>> >>>> The failure is:
>> >>> >>>>
>> >>> >>>> opt:
>> >>> >>>> /scratch/1/ismail/llvm-upstream/include/llvm/ADT/APInt.h:983:
>> >>> >>>> bool
>> >>> >>>> llvm::APInt::operator==(const llvm::APInt&) const: Assertion
>> >>> >>>> `BitWidth ==
>> >>> >>>> RHS.BitWidth && "Comparison requires equal bit widths"' failed.
>> >>> >>>> ...snip...
>> >>> >>>> #8 0x00000000013a8553 llvm::APInt::operator==(llvm::APInt const&)
>> >>> >>>> const
>> >>> >>>> /scratch/1/ismail/llvm-upstream/include/llvm/ADT/APInt.h:984:0
>> >>> >>>> #9 0x0000000001f875b0 simplifySelectBitTest(llvm::Value*,
>> >>> >>>> llvm::Value*,
>> >>> >>>> llvm::Value*, llvm::APInt const*, bool)
>> >>> >>>>
>> >>> >>>>
>> >>> >>>> /scratch/1/ismail/llvm-upstream/lib/Analysis/InstructionSimplify.cpp:3388:0
>> >>> >>>> #10 0x0000000001f87ad5 simplifySelectWithICmpCond(llvm::Value*,
>> >>> >>>> llvm::Value*, llvm::Value*, (anonymous namespace)::Query const&,
>> >>> >>>> unsigned
>> >>> >>>> int)
>> >>> >>>>
>> >>> >>>>
>> >>> >>>> /scratch/1/ismail/llvm-upstream/lib/Analysis/InstructionSimplify.cpp:3434:0
>> >>> >>>> #11 0x0000000001f87f6a SimplifySelectInst(llvm::Value*,
>> >>> >>>> llvm::Value*,
>> >>> >>>> llvm::Value*, (anonymous namespace)::Query const&, unsigned int)
>> >>> >>>>
>> >>> >>>>
>> >>> >>>> /scratch/1/ismail/llvm-upstream/lib/Analysis/InstructionSimplify.cpp:3515:0
>> >>> >>>> #12 0x0000000001f87fe3 llvm::SimplifySelectInst(llvm::Value*,
>> >>> >>>> llvm::Value*, llvm::Value*, llvm::DataLayout const&,
>> >>> >>>> llvm::TargetLibraryInfo
>> >>> >>>> const*, llvm::DominatorTree const*, llvm::AssumptionCache*,
>> >>> >>>> llvm::Instruction const*)
>> >>> >>>>
>> >>> >>>>
>> >>> >>>> /scratch/1/ismail/llvm-upstream/lib/Analysis/InstructionSimplify.cpp:3528:0
>> >>> >>>> ...snip...
>> >>> >>>> Stack dump:
>> >>> >>>> 0.  Program arguments: debugbuild/bin/opt -instcombine -licm
>> >>> >>>> -simplifycfg -instcombine -loop-rotate -loop-vectorize
>> >>> >>>> -instcombine
>> >>> >>>> 1.  Running pass 'Function Pass Manager' on module '<stdin>'.
>> >>> >>>> 2.  Running pass 'Combine redundant instructions' on function
>> >>> >>>> '@strsave'
>> >>> >>>>
>> >>> >>>> ---
>> >>> >>>>
>> >>> >>>> Looking at the code, the issue is with this line:
>> >>> >>>>
>> >>> >>>>   if (TrueVal == X && match(FalseVal, m_And(m_Specific(X),
>> >>> >>>> m_APInt(C)))
>> >>> >>>> &&
>> >>> >>>>         *Y == ~*C)
>> >>> >>>>
>> >>> >>>> In this case Y is a 128-bit APInt, and so is the value that C is
>> >>> >>>> extracted from, but the m_APInt matcher has code that calls
>> >>> >>>> getSplatValue()
>> >>> >>>> if the matched expression has vector type. So C ends up as an
>> >>> >>>> 8-bit
>> >>> >>>> value,
>> >>> >>>> triggering the assertion failure on the call to ==.
>> >>> >>>>
>> >>> >>>> The issue is clear but I'm not sure what the correct fix should
>> >>> >>>> be --
>> >>> >>>> whether this code is just not meant to work with vector types, or
>> >>> >>>> whether
>> >>> >>>> the call to getSplatValue() doesn't belong in the matcher, or
>> >>> >>>> whether
>> >>> >>>> there
>> >>> >>>> should be a corresponding call to getSplatValue() on the caller
>> >>> >>>> side,
>> >>> >>>> or
>> >>> >>>> something else.
>> >>> >>>>
>> >>> >>>> Any help from someone with more context would be appreciated.
>> >>> >>>>
>> >>> >>>> Thanks,
>> >>> >>>> Ismail
>> >>> >>>>
>> >>> >>>>
>> >>> >>>> _______________________________________________
>> >>> >>>> LLVM Developers mailing list
>> >>> >>>> llvm-dev at lists.llvm.org
>> >>> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> >>> >>>>
>> >>> >>>
>> >>> >>
>> >>> >
>> >>> >
>> >>> > _______________________________________________
>> >>> > LLVM Developers mailing list
>> >>> > llvm-dev at lists.llvm.org
>> >>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> >>> >
>> >>
>> >>