[llvm] r230355 - AArch64: Relax assert about large shift sizes.

Quentin Colombet qcolombet at apple.com
Wed Feb 25 09:28:34 PST 2015


Hi Matthias,

I’m still not convinced that this is the right fix but I can live with it.
However, could you pleas add some debug line when we hit the locations that were formerly the assertions (i.e., the returns false)?

That way, we know while debugging that there is undefined behavior that made its way in the backend.

Thanks,
-Quentin

> On Feb 24, 2015, at 10:52 AM, Matthias Braun <matze at braunis.de> wrote:
> 
> Author: matze
> Date: Tue Feb 24 12:52:04 2015
> New Revision: 230355
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=230355&view=rev
> Log:
> AArch64: Relax assert about large shift sizes.
> 
> The reason why these large shift sizes happen is because OpaqueConstants
> currently inhibit alot of DAG combining, but that has to be addressed in
> another commit (like the proposal in D6946).
> 
> Differential Revision: http://reviews.llvm.org/D6940
> 
> Added:
>    llvm/trunk/test/CodeGen/AArch64/large_shift.ll
> Modified:
>    llvm/trunk/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
> 
> Modified: llvm/trunk/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp?rev=230355&r1=230354&r2=230355&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp Tue Feb 24 12:52:04 2015
> @@ -1397,8 +1397,10 @@ static bool isBitfieldExtractOpFromAnd(S
>   } 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<uint32_t>(And_imm)
> @@ -1502,7 +1504,11 @@ static bool isBitfieldExtractOpFromShr(S
>   } else
>     return false;
> 
> -  assert(Shl_imm < VT.getSizeInBits() && "bad amount in shift node!");
> +  // Missing combines/constant folding may have left us with strange
> +  // constants.
> +  if (Shl_imm >= VT.getSizeInBits())
> +    return false;
> +
>   uint64_t Srl_imm = 0;
>   if (!isIntImmediate(N->getOperand(1), Srl_imm))
>     return false;
> 
> Added: llvm/trunk/test/CodeGen/AArch64/large_shift.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/large_shift.ll?rev=230355&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/large_shift.ll (added)
> +++ llvm/trunk/test/CodeGen/AArch64/large_shift.ll Tue Feb 24 12:52:04 2015
> @@ -0,0 +1,21 @@
> +; RUN: llc -march=aarch64 -o - %s
> +target triple = "arm64-unknown-unknown"
> +
> +; Make sure we don't run into an assert in the aarch64 code selection when
> +; DAGCombining fails.
> +
> +declare void @t()
> +
> +define void @foo() {
> +  %c = bitcast i64 270458 to i64
> +  %t0 = lshr i64 %c, 422383
> +  %t1 = trunc i64 %t0 to i1
> +  br i1 %t1, label %BB1, label %BB0
> +
> +BB0:
> +  call void @t()
> +  br label %BB1
> +
> +BB1:
> +  ret void
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits





More information about the llvm-commits mailing list