Hans Wennborg via llvm-dev
2016-Jul-22 13:45 UTC
[llvm-dev] Hitting assertion failure related to vectorization + instcombine
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 >
Sanjay Patel via llvm-dev
2016-Jul-22 14:08 UTC
[llvm-dev] Hitting assertion failure related to vectorization + instcombine
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()); <https://reviews.llvm.org/rL275911> 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 > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160722/568ceeab/attachment.html>
Hans Wennborg via llvm-dev
2016-Jul-25 18:07 UTC
[llvm-dev] Hitting assertion failure related to vectorization + instcombine
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 >> > > >