[llvm] r220700 - [FastISel][AArch64] Fix load/store with frame indices.

[Eric Christopher]

Hi Juergen,

Couple of small comments in line.

Thanks!

On Mon, Oct 27, 2014 at 11:21 AM, Juergen Ributzka <juergen at apple.com> wrote:
> Author: ributzka
> Date: Mon Oct 27 13:21:58 2014
> New Revision: 220700
>
> URL: http://llvm.org/viewvc/llvm-project?rev=220700&view=rev
> Log:
> [FastISel][AArch64] Fix load/store with frame indices.
>
> At higher optimization levels the LLVM IR may contain more complex patterns for
> loads/stores from/to frame indices. The 'computeAddress' function wasn't able to
> handle this and triggered an assertion.
>
> This fix extends the possible addressing modes for frame indices.
>
> This fixes rdar://problem/18783298.
>
> Modified:
>     llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp
>     llvm/trunk/test/CodeGen/AArch64/fast-isel-addressing-modes.ll
>
> Modified: llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp?rev=220700&r1=220699&r2=220700&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp Mon Oct 27 13:21:58 2014
> @@ -78,11 +78,9 @@ class AArch64FastISel final : public Fas
>        return Base.Reg;
>      }
>      void setOffsetReg(unsigned Reg) {
> -      assert(isRegBase() && "Invalid offset register access!");
>        OffsetReg = Reg;
>      }
>      unsigned getOffsetReg() const {
> -      assert(isRegBase() && "Invalid offset register access!");
>        return OffsetReg;
>      }
>      void setFI(unsigned FI) {
> @@ -810,22 +808,23 @@ bool AArch64FastISel::computeAddress(con
>    }
>    } // end switch
>
> -  if (Addr.getReg()) {
> -    if (!Addr.getOffsetReg()) {
> -      unsigned Reg = getRegForValue(Obj);
> -      if (!Reg)
> -        return false;
> -      Addr.setOffsetReg(Reg);
> -      return true;
> -    }
> -    return false;
> +  if (Addr.isRegBase() && !Addr.getReg()) {
> +    unsigned Reg = getRegForValue(Obj);
> +    if (!Reg)
> +      return false;
> +    Addr.setReg(Reg);
> +    return true;
>    }
>
> -  unsigned Reg = getRegForValue(Obj);
> -  if (!Reg)
> -    return false;
> -  Addr.setReg(Reg);
> -  return true;
> +  if (!Addr.getOffsetReg()) {
> +    unsigned Reg = getRegForValue(Obj);
> +    if (!Reg)
> +      return false;
> +    Addr.setOffsetReg(Reg);
> +    return true;
> +  }
> +
> +  return false;
>  }

Might be nice to comment out the address matching machinery as it's
going through the function.

>
>  bool AArch64FastISel::computeCallAddress(const Value *V, Address &Addr) {
> @@ -942,8 +941,7 @@ bool AArch64FastISel::simplifyAddress(Ad
>    // Cannot encode an offset register and an immediate offset in the same
>    // instruction. Fold the immediate offset into the load/store instruction and
>    // emit an additonal add to take care of the offset register.
> -  if (!ImmediateOffsetNeedsLowering && Addr.getOffset() && Addr.isRegBase() &&
> -      Addr.getOffsetReg())
> +  if (!ImmediateOffsetNeedsLowering && Addr.getOffset() && Addr.getOffsetReg())
>      RegisterOffsetNeedsLowering = true;
>
>    // Cannot encode zero register as base.
> @@ -953,7 +951,8 @@ bool AArch64FastISel::simplifyAddress(Ad
>    // If this is a stack pointer and the offset needs to be simplified then put
>    // the alloca address into a register, set the base type back to register and
>    // continue. This should almost never happen.
> -  if (ImmediateOffsetNeedsLowering && Addr.isFIBase()) {
> +  if ((ImmediateOffsetNeedsLowering || Addr.getOffsetReg()) && Addr.isFIBase())
> +  {
>

Formatting nit.

-eric

>  unsigned ResultReg = createResultReg(&AArch64::GPR64spRegClass);
>      BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(AArch64::ADDXri),
>              ResultReg)
> @@ -1050,10 +1049,8 @@ void AArch64FastISel::addLoadStoreOperan
>        MIB.addReg(Addr.getOffsetReg());
>        MIB.addImm(IsSigned);
>        MIB.addImm(Addr.getShift() != 0);
> -    } else {
> -      MIB.addReg(Addr.getReg());
> -      MIB.addImm(Offset);
> -    }
> +    } else
> +      MIB.addReg(Addr.getReg()).addImm(Offset);
>    }
>
>    if (MMO)
>
> Modified: llvm/trunk/test/CodeGen/AArch64/fast-isel-addressing-modes.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/fast-isel-addressing-modes.ll?rev=220700&r1=220699&r2=220700&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/fast-isel-addressing-modes.ll (original)
> +++ llvm/trunk/test/CodeGen/AArch64/fast-isel-addressing-modes.ll Mon Oct 27 13:21:58 2014
> @@ -599,3 +599,29 @@ define i64 @kill_reg(i64 %a) {
>    ret i64 %5
>  }
>
> +define void @store_fi(i64 %i) {
> +; CHECK-LABEL: store_fi
> +; CHECK:       mov [[REG:x[0-9]+]], sp
> +; CHECK:       str {{w[0-9]+}}, {{\[}}[[REG]], x0, lsl #2{{\]}}
> +  %1 = alloca [8 x i32]
> +  %2 = ptrtoint [8 x i32]* %1 to i64
> +  %3 = mul i64 %i, 4
> +  %4 = add i64 %2, %3
> +  %5 = inttoptr i64 %4 to i32*
> +  store i32 47, i32* %5, align 4
> +  ret void
> +}
> +
> +define i32 @load_fi(i64 %i) {
> +; CHECK-LABEL: load_fi
> +; CHECK:       mov [[REG:x[0-9]+]], sp
> +; CHECK:       ldr {{w[0-9]+}}, {{\[}}[[REG]], x0, lsl #2{{\]}}
> +  %1 = alloca [8 x i32]
> +  %2 = ptrtoint [8 x i32]* %1 to i64
> +  %3 = mul i64 %i, 4
> +  %4 = add i64 %2, %3
> +  %5 = inttoptr i64 %4 to i32*
> +  %6 = load i32* %5, align 4
> +  ret i32 %6
> +}
> +
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits