[llvm] r214345 - [FastISel][AArch64] Add support for shift-immediate.

Chad Rosier mcrosier at codeaurora.org
Sat Aug 2 20:39:50 PDT 2014


Hi Juergen,
A shift with a non-legal value type (i.e., i8/i16) requires special
handling.  Consider the following testcase:

target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
target triple = "aarch64--linux-gnu"

@C = constant i8 65, align 4

define i32 @main(i32 %argc, i8** %argv) {
entry:
  %load = load i8* @C, align 1
  %shl = shl i8 %load, 4
  %ashr = ashr i8 %shl, 4
  %cast = sext i8 %ashr to i32
  ret i32 %cast
}

Fast-isel produces 65, but the correct value is 1.  The first solution
that comes to mind would be to emit an ANDri with the appropriate mask
after a left-shift.

When you have a moment, please take a look..

 Regards,
  Chad

> Author: ributzka
> Date: Wed Jul 30 17:04:22 2014
> New Revision: 214345
>
> URL: http://llvm.org/viewvc/llvm-project?rev=214345&view=rev
> Log:
> [FastISel][AArch64] Add support for shift-immediate.
>
> Currently the shift-immediate versions are not supported by tblgen and
> hopefully this can be later removed, once the required support has been
> added to tblgen.
>
> Added:
>     llvm/trunk/test/CodeGen/AArch64/fast-isel-shift.ll
> Modified:
>     llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp
>
> Modified: llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp?rev=214345&r1=214344&r2=214345&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp Wed Jul 30 17:04:22
> 2014
> @@ -109,6 +109,7 @@ private:
>    bool SelectTrunc(const Instruction *I);
>    bool SelectIntExt(const Instruction *I);
>    bool SelectMul(const Instruction *I);
> +  bool SelectShift(const Instruction *I, bool IsLeftShift, bool
> IsArithmetic);
>
>    // Utility helper routines.
>    bool isTypeLegal(Type *Ty, MVT &VT);
> @@ -129,6 +130,9 @@ private:
>                   bool UseUnscaled = false);
>    unsigned EmitIntExt(MVT SrcVT, unsigned SrcReg, MVT DestVT, bool
> isZExt);
>    unsigned Emiti1Ext(unsigned SrcReg, MVT DestVT, bool isZExt);
> +  unsigned Emit_LSL_ri(MVT RetVT, unsigned Op0, bool Op0IsKill, uint64_t
> Imm);
> +  unsigned Emit_LSR_ri(MVT RetVT, unsigned Op0, bool Op0IsKill, uint64_t
> Imm);
> +  unsigned Emit_ASR_ri(MVT RetVT, unsigned Op0, bool Op0IsKill, uint64_t
> Imm);
>
>    unsigned AArch64MaterializeFP(const ConstantFP *CFP, MVT VT);
>    unsigned AArch64MaterializeGV(const GlobalValue *GV);
> @@ -1722,6 +1726,60 @@ unsigned AArch64FastISel::Emiti1Ext(unsi
>    }
>  }
>
> +unsigned AArch64FastISel::Emit_LSL_ri(MVT RetVT, unsigned Op0, bool
> Op0IsKill,
> +                                      uint64_t Shift) {
> +  unsigned Opc, ImmR, ImmS;
> +  switch (RetVT.SimpleTy) {
> +  default: return 0;
> +  case MVT::i8:
> +  case MVT::i16:
> +  case MVT::i32:
> +    RetVT = MVT::i32;
> +    Opc = AArch64::UBFMWri; ImmR = -Shift % 32; ImmS = 31 - Shift; break;
> +  case MVT::i64:
> +    Opc = AArch64::UBFMXri; ImmR = -Shift % 64; ImmS = 63 - Shift; break;
> +  }
> +
> +  return FastEmitInst_rii(Opc, TLI.getRegClassFor(RetVT), Op0, Op0IsKill,
> ImmR,
> +                          ImmS);
> +}
> +
> +unsigned AArch64FastISel::Emit_LSR_ri(MVT RetVT, unsigned Op0, bool
> Op0IsKill,
> +                                      uint64_t Shift) {
> +  unsigned Opc, ImmS;
> +  switch (RetVT.SimpleTy) {
> +  default: return 0;
> +  case MVT::i8:
> +  case MVT::i16:
> +  case MVT::i32:
> +    RetVT = MVT::i32;
> +    Opc = AArch64::UBFMWri; ImmS = 31; break;
> +  case MVT::i64:
> +    Opc = AArch64::UBFMXri; ImmS = 63; break;
> +  }
> +
> +  return FastEmitInst_rii(Opc, TLI.getRegClassFor(RetVT), Op0, Op0IsKill,
> Shift,
> +                          ImmS);
> +}
> +
> +unsigned AArch64FastISel::Emit_ASR_ri(MVT RetVT, unsigned Op0, bool
> Op0IsKill,
> +                                      uint64_t Shift) {
> +  unsigned Opc, ImmS;
> +  switch (RetVT.SimpleTy) {
> +  default: return 0;
> +  case MVT::i8:
> +  case MVT::i16:
> +  case MVT::i32:
> +    RetVT = MVT::i32;
> +    Opc = AArch64::SBFMWri; ImmS = 31; break;
> +  case MVT::i64:
> +    Opc = AArch64::SBFMXri; ImmS = 63; break;
> +  }
> +
> +  return FastEmitInst_rii(Opc, TLI.getRegClassFor(RetVT), Op0, Op0IsKill,
> Shift,
> +                          ImmS);
> +}
> +
>  unsigned AArch64FastISel::EmitIntExt(MVT SrcVT, unsigned SrcReg, MVT
> DestVT,
>                                       bool isZExt) {
>    assert(DestVT != MVT::i1 && "ZeroExt/SignExt an i1?");
> @@ -1908,6 +1966,40 @@ bool AArch64FastISel::SelectMul(const In
>    return true;
>  }
>
> +bool AArch64FastISel::SelectShift(const Instruction *I, bool IsLeftShift,
> +                                  bool IsArithmetic) {
> +  EVT RetEVT = TLI.getValueType(I->getType(), true);
> +  if (!RetEVT.isSimple())
> +    return false;
> +  MVT RetVT = RetEVT.getSimpleVT();
> +
> +  if (!isa<ConstantInt>(I->getOperand(1)))
> +    return false;
> +
> +  unsigned Op0Reg = getRegForValue(I->getOperand(0));
> +  if (!Op0Reg)
> +    return false;
> +  bool Op0IsKill = hasTrivialKill(I->getOperand(0));
> +
> +  uint64_t ShiftVal =
> cast<ConstantInt>(I->getOperand(1))->getZExtValue();
> +
> +  unsigned ResultReg;
> +  if (IsLeftShift)
> +    ResultReg = Emit_LSL_ri(RetVT, Op0Reg, Op0IsKill, ShiftVal);
> +  else {
> +    if (IsArithmetic)
> +      ResultReg = Emit_ASR_ri(RetVT, Op0Reg, Op0IsKill, ShiftVal);
> +    else
> +      ResultReg = Emit_LSR_ri(RetVT, Op0Reg, Op0IsKill, ShiftVal);
> +  }
> +
> +  if (!ResultReg)
> +    return false;
> +
> +  UpdateValueMap(I, ResultReg);
> +  return true;
> +}
> +
>  bool AArch64FastISel::TargetSelectInstruction(const Instruction *I) {
>    switch (I->getOpcode()) {
>    default:
> @@ -1948,9 +2040,17 @@ bool AArch64FastISel::TargetSelectInstru
>    case Instruction::ZExt:
>    case Instruction::SExt:
>      return SelectIntExt(I);
> +
> +  // FIXME: All of these should really be handled by the
> target-independent
> +  // selector -> improve FastISel tblgen.
>    case Instruction::Mul:
> -    // FIXME: This really should be handled by the target-independent
> selector.
>      return SelectMul(I);
> +  case Instruction::Shl:
> +      return SelectShift(I, /*IsLeftShift=*/true,
> /*IsArithmetic=*/false);
> +  case Instruction::LShr:
> +    return SelectShift(I, /*IsLeftShift=*/false, /*IsArithmetic=*/false);
> +  case Instruction::AShr:
> +    return SelectShift(I, /*IsLeftShift=*/false, /*IsArithmetic=*/true);
>    }
>    return false;
>    // Silence warnings.
>
> Added: llvm/trunk/test/CodeGen/AArch64/fast-isel-shift.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/fast-isel-shift.ll?rev=214345&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/fast-isel-shift.ll (added)
> +++ llvm/trunk/test/CodeGen/AArch64/fast-isel-shift.ll Wed Jul 30 17:04:22
> 2014
> @@ -0,0 +1,89 @@
> +; RUN: llc -fast-isel -fast-isel-abort -mtriple=arm64-apple-darwin < %s |
> FileCheck %s
> +
> +; CHECK-LABEL: lsl_i8
> +; CHECK: lsl {{w[0-9]*}}, {{w[0-9]*}}, #4
> +define zeroext i8 @lsl_i8(i8 %a) {
> +  %1 = shl i8 %a, 4
> +  ret i8 %1
> +}
> +
> +; CHECK-LABEL: lsl_i16
> +; CHECK: lsl {{w[0-9]*}}, {{w[0-9]*}}, #8
> +define zeroext i16 @lsl_i16(i16 %a) {
> +  %1 = shl i16 %a, 8
> +  ret i16 %1
> +}
> +
> +; CHECK-LABEL: lsl_i32
> +; CHECK: lsl {{w[0-9]*}}, {{w[0-9]*}}, #16
> +define zeroext i32 @lsl_i32(i32 %a) {
> +  %1 = shl i32 %a, 16
> +  ret i32 %1
> +}
> +
> +; FIXME: This shouldn't use the variable shift version.
> +; CHECK-LABEL: lsl_i64
> +; CHECK: lsl {{x[0-9]*}}, {{x[0-9]*}}, {{x[0-9]*}}
> +define i64 @lsl_i64(i64 %a) {
> +  %1 = shl i64 %a, 32
> +  ret i64 %1
> +}
> +
> +; CHECK-LABEL: lsr_i8
> +; CHECK: lsr {{w[0-9]*}}, {{w[0-9]*}}, #4
> +define zeroext i8 @lsr_i8(i8 %a) {
> +  %1 = lshr i8 %a, 4
> +  ret i8 %1
> +}
> +
> +; CHECK-LABEL: lsr_i16
> +; CHECK: lsr {{w[0-9]*}}, {{w[0-9]*}}, #8
> +define zeroext i16 @lsr_i16(i16 %a) {
> +  %1 = lshr i16 %a, 8
> +  ret i16 %1
> +}
> +
> +; CHECK-LABEL: lsr_i32
> +; CHECK: lsr {{w[0-9]*}}, {{w[0-9]*}}, #16
> +define zeroext i32 @lsr_i32(i32 %a) {
> +  %1 = lshr i32 %a, 16
> +  ret i32 %1
> +}
> +
> +; FIXME: This shouldn't use the variable shift version.
> +; CHECK-LABEL: lsr_i64
> +; CHECK: lsr {{x[0-9]*}}, {{x[0-9]*}}, {{x[0-9]*}}
> +define i64 @lsr_i64(i64 %a) {
> +  %1 = lshr i64 %a, 32
> +  ret i64 %1
> +}
> +
> +; CHECK-LABEL: asr_i8
> +; CHECK: asr {{w[0-9]*}}, {{w[0-9]*}}, #4
> +define zeroext i8 @asr_i8(i8 %a) {
> +  %1 = ashr i8 %a, 4
> +  ret i8 %1
> +}
> +
> +; CHECK-LABEL: asr_i16
> +; CHECK: asr {{w[0-9]*}}, {{w[0-9]*}}, #8
> +define zeroext i16 @asr_i16(i16 %a) {
> +  %1 = ashr i16 %a, 8
> +  ret i16 %1
> +}
> +
> +; CHECK-LABEL: asr_i32
> +; CHECK: asr {{w[0-9]*}}, {{w[0-9]*}}, #16
> +define zeroext i32 @asr_i32(i32 %a) {
> +  %1 = ashr i32 %a, 16
> +  ret i32 %1
> +}
> +
> +; FIXME: This shouldn't use the variable shift version.
> +; CHECK-LABEL: asr_i64
> +; CHECK: asr {{x[0-9]*}}, {{x[0-9]*}}, {{x[0-9]*}}
> +define i64 @asr_i64(i64 %a) {
> +  %1 = ashr i64 %a, 32
> +  ret i64 %1
> +}
> +
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation




More information about the llvm-commits mailing list