[PATCH] AArch64: Relax assert about large shift sizes.

[Ahmed Bougacha]

On Tue, Jan 13, 2015 at 1:34 AM, Matthias Braun <matze at braunis.de> wrote:
> I think replacing it with undef would be legal, but isn't replacing this
> with undef a combine/constant fold thing? Should we duplicate that logic in
> the Selection DAG? I'm new to this part of llvm, so maybe I don't understand
> the philosophy here yet.

The SelectionDAG has its own combiner (see
lib/CodeGen/SelectionDAG/DAGCombiner.cpp).

A quick examination of visitSRL there shows the undef folding is after
the both-constants folding, which I guess fails for some reason in
this case (probably the bitcast).

In any case, I think the change in your patch is OK; Quentin, what do you think?

-Ahmed

> Greetings
> Matthias
>
> On Jan 12, 2015, at 4:14 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>
> Hi Matthias,
>
> I am wondering if we shouldn’t generate an undef node in Selection DAG for
> such shifts, unless the semantic is properly defined there.
>
> What do you think?
>
>
> On Jan 12, 2015, at 3:28 PM, Matthias Braun <matze at braunis.de> wrote:
>
> Shift sizes larger than the size of the value in bits can happen if
> constant folding fails.
>
> Testcase based on rdar://19211454
>
> http://reviews.llvm.org/D6940
>
> Files:
> lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
> test/CodeGen/AArch64/large_shift.ll
>
> Index: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
> ===================================================================
> --- lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
> +++ lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
> @@ -1424,8 +1424,10 @@
>  } else
>    return false;
>
> -  assert((BiggerPattern || (Srl_imm > 0 && Srl_imm < VT.getSizeInBits()))
> &&
> -         "bad amount in shift node!");
> +  // Bail out on large immediates. This happens when no proper
> +  // combining/constant folding was performed.
> +  if (!BiggerPattern && (Srl_imm <= 0 || Srl_imm >= VT.getSizeInBits()))
> +    return false;
>
>  LSB = Srl_imm;
>  MSB = Srl_imm + (VT == MVT::i32 ? CountTrailingOnes_32(And_imm)
> Index: test/CodeGen/AArch64/large_shift.ll
> ===================================================================
> --- /dev/null
> +++ test/CodeGen/AArch64/large_shift.ll
> @@ -0,0 +1,18 @@
> +; RUN: llc -march=aarch64 -o - %s
> +target triple = "arm64-unknown-unknown"
> +
> +declare void @t()
> +
> +define void @foo() {
> +  %c = bitcast i64 270458 to i64
> +  %t0 = lshr i64 %c, 422383
>
>
> What assembly is generated for this?
>
> Thanks,
> -Quentin
>
> +  %t1 = trunc i64 %t0 to i1
> +  br i1 %t1, label %BB1, label %BB0
> +
> +BB0:
> +  call void @t()
> +  br label %BB1
> +
> +BB1:
> +  ret void
> +}
>
> EMAIL PREFERENCES
> http://reviews.llvm.org/settings/panel/emailpreferences/
> <D6940.18054.patch>_______________________________________________
> 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
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>