LGTM<br><br>On Wednesday, July 27, 2016, Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">David, Sanjay: ping?<br>
<br>
On Mon, Jul 25, 2016 at 11:07 AM, Hans Wennborg <<a href="javascript:;" onclick="_e(event, 'cvml', 'hans@chromium.org')">hans@chromium.org</a>> wrote:<br>
> Sure. David, what do you think about merging this to 3.9?<br>
><br>
> Sanjay: are you saying I'd just apply that diff to<br>
> InstructionSimplify.cpp, not InstCombineSelect.cpp?<br>
><br>
> On Fri, Jul 22, 2016 at 7:08 AM, Sanjay Patel <<a href="javascript:;" onclick="_e(event, 'cvml', 'spatel@rotateright.com')">spatel@rotateright.com</a>> wrote:<br>
>> Hi Hans -<br>
>><br>
>> Yes, I think this is a good patch for 3.9 (cc'ing David Majnemer as code<br>
>> owner). The functional change was r276209. I made an NFC refactoring in<br>
>> InstSimplify at r275911 that isn't in the branch, so I don't think the patch<br>
>> will apply as-is. For safety, you could apply the one-line fix without<br>
>> 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>
>><br>
>><br>
>> On Fri, Jul 22, 2016 at 7:45 AM, Hans Wennborg <<a href="javascript:;" onclick="_e(event, 'cvml', 'hans@chromium.org')">hans@chromium.org</a>> wrote:<br>
>>><br>
>>> 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="javascript:;" onclick="_e(event, 'cvml', 'llvm-dev@lists.llvm.org')">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" target="_blank">https://reviews.llvm.org/rL275911</a><br>
>>> ><br>
>>> > In fact, as discussed in <a href="https://reviews.llvm.org/D22537" target="_blank">https://reviews.llvm.org/D22537</a> - because we<br>
>>> > have a<br>
>>> > big pile of lazily copied code, the bug has an identical twin that<br>
>>> > 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 2147483648, i32 2147483648><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<br>
>>> > 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="javascript:;" onclick="_e(event, 'cvml', 'spatel@rotateright.com')">spatel@rotateright.com</a>><br>
>>> > wrote:<br>
>>> >><br>
>>> >> Thanks for notifying me. Yes, this was a recent change. Taking a look<br>
>>> >> now.<br>
>>> >><br>
>>> >> On Wed, Jul 20, 2016 at 1:49 PM, Michael Kuperstein <<a href="javascript:;" onclick="_e(event, 'cvml', 'mkuper@google.com')">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="javascript:;" onclick="_e(event, 'cvml', 'llvm-dev@lists.llvm.org')">llvm-dev@lists.llvm.org</a>> wrote:<br>
>>> >>>><br>
>>> >>>> Hi folks,<br>
>>> >>>><br>
>>> >>>> I'm hitting the below assertion failure when compiling this small<br>
>>> >>>> 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<br>
>>> >>>> at<br>
>>> >>>> -O2, and it doesn't happen with the default target (x86_64). I tried<br>
>>> >>>> to<br>
>>> >>>> reproduce using just 'llc -O2' but didn't manage -- but I do have<br>
>>> >>>> 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:<br>
>>> >>>> bool<br>
>>> >>>> llvm::APInt::operator==(const llvm::APInt&) const: Assertion<br>
>>> >>>> `BitWidth ==<br>
>>> >>>> RHS.BitWidth && "Comparison requires equal bit widths"' failed.<br>
>>> >>>> ...snip...<br>
>>> >>>> #8 0x00000000013a8553 llvm::APInt::operator==(llvm::APInt const&)<br>
>>> >>>> const<br>
>>> >>>> /scratch/1/ismail/llvm-upstream/include/llvm/ADT/APInt.h:984:0<br>
>>> >>>> #9 0x0000000001f875b0 simplifySelectBitTest(llvm::Value*,<br>
>>> >>>> llvm::Value*,<br>
>>> >>>> llvm::Value*, llvm::APInt const*, bool)<br>
>>> >>>><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&,<br>
>>> >>>> unsigned<br>
>>> >>>> int)<br>
>>> >>>><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>
>>> >>>><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&,<br>
>>> >>>> llvm::TargetLibraryInfo<br>
>>> >>>> const*, llvm::DominatorTree const*, llvm::AssumptionCache*,<br>
>>> >>>> llvm::Instruction const*)<br>
>>> >>>><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<br>
>>> >>>> '@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),<br>
>>> >>>> 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<br>
>>> >>>> getSplatValue()<br>
>>> >>>> if the matched expression has vector type. So C ends up as an 8-bit<br>
>>> >>>> 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<br>
>>> >>>> whether<br>
>>> >>>> the call to getSplatValue() doesn't belong in the matcher, or whether<br>
>>> >>>> there<br>
>>> >>>> should be a corresponding call to getSplatValue() on the caller side,<br>
>>> >>>> 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="javascript:;" onclick="_e(event, 'cvml', 'llvm-dev@lists.llvm.org')">llvm-dev@lists.llvm.org</a><br>
>>> >>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" 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="javascript:;" onclick="_e(event, 'cvml', 'llvm-dev@lists.llvm.org')">llvm-dev@lists.llvm.org</a><br>
>>> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>>> ><br>
>><br>
>><br>
</blockquote>