[llvm] r243100 - When lowering vector shifts a check is performed to see if the value to shift by

[Justin Bogner]

Luke Cheeseman <luke.cheeseman at arm.com> writes:
> Author: lukecheeseman
> Date: Fri Jul 24 04:31:48 2015
> New Revision: 243100
>
> URL: http://llvm.org/viewvc/llvm-project?rev=243100&view=rev
> Log:
> When lowering vector shifts a check is performed to see if the value to shift by
> is an immediate, in this check the value is negated and stored in and int64_t.
> The value can be -2^63 yet the result cannot be stored in an int64_t and this
> gives some undefined behaviour causing failures. The negation is only necessary
> when the values is within a certain range and so it should not need to negate
> -2^63, this patch introduces this and also a regression test.
>
> Differential Revision: http://reviews.llvm.org/D11408
>
>
> Added:
>     llvm/trunk/test/CodeGen/ARM/neon_vshl_minint.ll
> Modified:
>     llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp
>     llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
>
> Modified: llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp?rev=243100&r1=243099&r2=243100&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp Fri Jul 24 04:31:48 2015
> @@ -6440,26 +6440,20 @@ static bool getVShiftImm(SDValue Op, uns
>  ///   0 <= Value <= ElementBits for a long left shift.
>  static bool isVShiftLImm(SDValue Op, EVT VT, bool isLong, int64_t &Cnt) {
>    assert(VT.isVector() && "vector shift count is not a vector type");
> -  unsigned ElementBits = VT.getVectorElementType().getSizeInBits();
> +  int64_t ElementBits = VT.getVectorElementType().getSizeInBits();
>    if (!getVShiftImm(Op, ElementBits, Cnt))
>      return false;
>    return (Cnt >= 0 && (isLong ? Cnt - 1 : Cnt) < ElementBits);
>  }
>  
>  /// isVShiftRImm - Check if this is a valid build_vector for the immediate
> -/// operand of a vector shift right operation.  For a shift opcode, the value
> -/// is positive, but for an intrinsic the value count must be negative. The
> -/// absolute value must be in the range:
> -///   1 <= |Value| <= ElementBits for a right shift; or
> -///   1 <= |Value| <= ElementBits/2 for a narrow right shift.
> -static bool isVShiftRImm(SDValue Op, EVT VT, bool isNarrow, bool isIntrinsic,
> -                         int64_t &Cnt) {
> +/// operand of a vector shift right operation. The value must be in the range:
> +///   1 <= Value <= ElementBits for a right shift; or

Sentences don't usually end in `or`, I guess something got cut off here.

> +static bool isVShiftRImm(SDValue Op, EVT VT, bool isNarrow, int64_t &Cnt) {
>    assert(VT.isVector() && "vector shift count is not a vector type");
> -  unsigned ElementBits = VT.getVectorElementType().getSizeInBits();
> +  int64_t ElementBits = VT.getVectorElementType().getSizeInBits();
>    if (!getVShiftImm(Op, ElementBits, Cnt))
>      return false;
> -  if (isIntrinsic)
> -    Cnt = -Cnt;
>    return (Cnt >= 1 && Cnt <= (isNarrow ? ElementBits / 2 : ElementBits));
>  }
>  
> @@ -6488,8 +6482,7 @@ SDValue AArch64TargetLowering::LowerVect
>    case ISD::SRA:
>    case ISD::SRL:
>      // Right shift immediate
> -    if (isVShiftRImm(Op.getOperand(1), VT, false, false, Cnt) &&
> -        Cnt < EltSize) {
> +    if (isVShiftRImm(Op.getOperand(1), VT, false, Cnt) && Cnt < EltSize) {
>        unsigned Opc =
>            (Op.getOpcode() == ISD::SRA) ? AArch64ISD::VASHR : AArch64ISD::VLSHR;
>        return DAG.getNode(Opc, DL, VT, Op.getOperand(0),
>
> Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=243100&r1=243099&r2=243100&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Fri Jul 24 04:31:48 2015
> @@ -9691,7 +9691,7 @@ static bool getVShiftImm(SDValue Op, uns
>  ///   0 <= Value <= ElementBits for a long left shift.
>  static bool isVShiftLImm(SDValue Op, EVT VT, bool isLong, int64_t &Cnt) {
>    assert(VT.isVector() && "vector shift count is not a vector type");
> -  unsigned ElementBits = VT.getVectorElementType().getSizeInBits();
> +  int64_t ElementBits = VT.getVectorElementType().getSizeInBits();
>    if (! getVShiftImm(Op, ElementBits, Cnt))
>      return false;
>    return (Cnt >= 0 && (isLong ? Cnt-1 : Cnt) < ElementBits);
> @@ -9706,12 +9706,16 @@ static bool isVShiftLImm(SDValue Op, EVT
>  static bool isVShiftRImm(SDValue Op, EVT VT, bool isNarrow, bool isIntrinsic,
>                           int64_t &Cnt) {
>    assert(VT.isVector() && "vector shift count is not a vector type");
> -  unsigned ElementBits = VT.getVectorElementType().getSizeInBits();
> +  int64_t ElementBits = VT.getVectorElementType().getSizeInBits();
>    if (! getVShiftImm(Op, ElementBits, Cnt))
>      return false;
> -  if (isIntrinsic)
> +  if (!isIntrinsic)
> +    return (Cnt >= 1 && Cnt <= (isNarrow ? ElementBits/2 : ElementBits));
> +  if (Cnt >= -(isNarrow ? ElementBits/2 : ElementBits) && Cnt <= -1) {
>      Cnt = -Cnt;
> -  return (Cnt >= 1 && Cnt <= (isNarrow ? ElementBits/2 : ElementBits));
> +    return true;
> +  }
> +  return false;
>  }
>  
>  /// PerformIntrinsicCombine - ARM-specific DAG combining for intrinsics.
>
> Added: llvm/trunk/test/CodeGen/ARM/neon_vshl_minint.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/neon_vshl_minint.ll?rev=243100&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/neon_vshl_minint.ll (added)
> +++ llvm/trunk/test/CodeGen/ARM/neon_vshl_minint.ll Fri Jul 24 04:31:48 2015
> @@ -0,0 +1,13 @@
> +; RUN: llc < %s -mtriple=arm-none-eabi -mcpu=cortex-a8 2>&1 | FileCheck %s
> +; RUN: llc < %s -mtriple=thumb-none-eabi -mcpu=cortex-a8 2>&1 | FileCheck %s
> +
> +define <1 x i64> @vshl_minint() #0 {
> +  entry:
> +    ; CHECK-LABEL: vshl_minint
> +    ; CHECK: vldr
> +    ; CHECK: vshl.u64
> +    %vshl.i = tail call <1 x i64> @llvm.arm.neon.vshiftu.v1i64(<1 x i64> undef, <1 x i64> <i64 -9223372036854775808>)
> +    ret <1 x i64> %vshl.i
> +}
> +
> +declare <1 x i64> @llvm.arm.neon.vshiftu.v1i64(<1 x i64>, <1 x i64>)
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits