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