[llvm] r240291 - Fix shl folding in DAG combiner.

[Paweł Bylica]

That's it, thanks for checking that.

Looks like APInt::uge first truncs the uint64_t value to it's size and then
compares. In this case it will trunc 256 to i8 what is 0.

Paweł

On Tue, Jun 23, 2015 at 3:15 AM Frédéric Riss <friss at apple.com> wrote:

>
> On Jun 22, 2015, at 3:08 PM, Frédéric Riss <friss at apple.com> wrote:
>
>
> On Jun 22, 2015, at 2:59 PM, Paweł Bylica <chfast at gmail.com> wrote:
>
> Reverted in r240343. I'm finishing for today. Sorry for the troubles, but
> it's quite a surprise for me.
>
>
> Honestly, nothing obvious seems wrong with the commit, so I understand
> your surprise, but the facts seems to show otherwise :-/
>
> How can I reproduce the problem locally?
>
>
> I would guess that recompiling a clang using a clang that has your change
> should expose the issue (that’s what I would expect the
> clang-x86_64-linux-selfhost-abi-test does. I’ll try it myself and see if I
> can get you some data.
>
>
> Your revert seems to have fixed the bots.
>
> I compiled a clang with a clang that I instrumented to see where your new
> version would produce a different results that the old one. Here’s one case
> I found:
>
> (lldb) p N1C->dump()
> 0x11a9666b0: i8 = Constant<-64>
> (lldb) p OpSizeInBits
> (unsigned int) $1 = 256
>
> It happens when compiling:
> clang::ASTReader::ParseDiagnosticOptions(llvm::SmallVector<unsigned long
> long, 64u> const&, bool, clang::ASTReaderListener&)
> in <clang>/lib/Serialization/ASTReader.cpp
>
> Hope this helps you find what is wrong.
>
> Fred
>
>
> Fred
>
> On Mon, Jun 22, 2015 at 11:38 PM Frédéric Riss <friss at apple.com> wrote:
>
> On Jun 22, 2015, at 1:57 PM, Paweł Bylica <chfast at gmail.com> wrote:
>
> I'm about to send a change that will fix the regression test (it should be
> -march=x86). It should fix build failures like:
> http://lab.llvm.org:8011/builders/llvm-s390x-linux1/builds/16458
>
> However your is similar to that one:
>
> http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-abi-test/builds/5852
> but I'm not sure what's wrong with that.
>
> Give me 10 more minutes. If it will not help, please revert it.
>
>
> I just saw your commit. I think there is no way that your fix in the test
> could fix any of the self host issues (the green dragon failure is a
> self-hosted LTO build and the other failing bot you pointed me at is also a
> self-host). Are you investigating the other failures? The 2 failing bots
> have only your commit in common in their blame lists so at this point it’s
> pretty clear that this commit triggers something very wrong in a clang
> self-host build. It  should be pretty easy to reproduce. I’d appreciate if
> you revert this change so that the bots can become green again.
>
> Fred
>
> - Paweł
>
>
>
> On Mon, Jun 22, 2015 at 10:47 PM Frédéric Riss <friss at apple.com> wrote:
> This commit is by far the most likely culprit for the hangs we see on
> green dragon:
> http://lab.llvm.org:8080/green/job/clang-stage2-configure-Rlto_check/
>
> Did you have any other failures? Would you mind reverting it to see if it
> fixes things? (This is after an LTO bootstrap, so it takes quite some time
> to reproduce/propagate).
>
> Fred
>
> On Jun 22, 2015, at 8:58 AM, Pawel Bylica <chfast at gmail.com> wrote:
>
> Author: chfast
> Date: Mon Jun 22 10:58:11 2015
> New Revision: 240291
>
> URL: http://llvm.org/viewvc/llvm-project?rev=240291&view=rev
> Log:
> Fix shl folding in DAG combiner.
>
> Summary: The code responsible for shl folding in the DAGCombiner was
> assuming incorrectly that all constants are less than 64 bits. This patch
> simply changes the way values are compared.
>
> Test Plan: A regression test included.
>
> Reviewers: andreadb
>
> Reviewed By: andreadb
>
> Subscribers: andreadb, test, llvm-commits
>
> Differential Revision: http://reviews.llvm.org/D10602
>
> Added:
>   llvm/trunk/test/CodeGen/X86/fold-vector-shl-crash.ll
> Modified:
>   llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=240291&r1=240290&r2=240291&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon Jun 22
> 10:58:11 2015
> @@ -4275,7 +4275,7 @@ SDValue DAGCombiner::visitSHL(SDNode *N)
>  if (isNullConstant(N0))
>    return N0;
>  // fold (shl x, c >= size(x)) -> undef
> -  if (N1C && N1C->getZExtValue() >= OpSizeInBits)
> +  if (N1C && N1C->getAPIntValue().uge(OpSizeInBits))
>    return DAG.getUNDEF(VT);
>  // fold (shl x, 0) -> x
>  if (N1C && N1C->isNullValue())
>
> Added: llvm/trunk/test/CodeGen/X86/fold-vector-shl-crash.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fold-vector-shl-crash.ll?rev=240291&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/fold-vector-shl-crash.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/fold-vector-shl-crash.ll Mon Jun 22
> 10:58:11 2015
> @@ -0,0 +1,8 @@
> +; RUN: llc < %s | FileCheck %s
> +
> +;CHECK-LABEL: test
> +define <2 x i256> @test() {
> +  %S = shufflevector <2 x i256> zeroinitializer, <2 x i256> <i256 -1,
> i256 -1>, <2 x i32> <i32 0, i32 2>
> +  %B = shl <2 x i256> %S, <i256 -1, i256 -1> ; DAG Combiner crashes here
> +  ret <2 x i256> %B
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150623/419dc7da/attachment.html>