[llvm-commits] [llvm] r156458 - in /llvm/trunk: lib/Target/Mips/MipsRegisterInfo.cpp test/CodeGen/Mips/fp-spill-reload.ll

[Jim Grosbach]

Hi Akira,

Does the Mips backend ever do dynamic stack realignment for register spills? If so, there are additional complexities to consider as whether the FP is required can change after regalloc. ARM and X86 run into that problem, for example.

One nitpick below.

Thanks,
  Jim

On May 8, 2012, at 6:38 PM, Akira Hatanaka wrote:

> Author: ahatanak
> Date: Tue May  8 20:38:13 2012
> New Revision: 156458
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=156458&view=rev
> Log:
> Make register FP allocatable if the compiled function does not have dynamic
> allocas.
> 
> 
> Added:
>    llvm/trunk/test/CodeGen/Mips/fp-spill-reload.ll
> Modified:
>    llvm/trunk/lib/Target/Mips/MipsRegisterInfo.cpp
> 
> Modified: llvm/trunk/lib/Target/Mips/MipsRegisterInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsRegisterInfo.cpp?rev=156458&r1=156457&r2=156458&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/Mips/MipsRegisterInfo.cpp (original)
> +++ llvm/trunk/lib/Target/Mips/MipsRegisterInfo.cpp Tue May  8 20:38:13 2012
> @@ -85,12 +85,12 @@
> getReservedRegs(const MachineFunction &MF) const {
>   static const uint16_t ReservedCPURegs[] = {
>     Mips::ZERO, Mips::AT, Mips::K0, Mips::K1,
> -    Mips::SP, Mips::FP, Mips::RA
> +    Mips::SP, Mips::RA
>   };
> 
>   static const uint16_t ReservedCPU64Regs[] = {
>     Mips::ZERO_64, Mips::AT_64, Mips::K0_64, Mips::K1_64,
> -    Mips::SP_64, Mips::FP_64, Mips::RA_64
> +    Mips::SP_64, Mips::RA_64
>   };
> 
>   BitVector Reserved(getNumRegs());
> @@ -124,6 +124,12 @@
>     Reserved.set(Mips::GP_64);
>   }
> 
> +  // If this function has dynamic allocas, reserve FP.
> +  if (MF.getTarget().getFrameLowering()->hasFP(MF)) {

The code doesn't seem to, at least directly, follow from the comment. Based on the comment I'd have expected to see MFI->hasVarSizedObjects(). That said, I think it's the comment that's misleading, not the code. Are there ever cases where the function has a frame pointer, whether vararray related or not, that the FP shouldn't be reserved? I would think not, so that's probably what the comment say and not reference vararrays at all.

> +    Reserved.set(Mips::FP);
> +    Reserved.set(Mips::FP_64);
> +  }
> +
>   // Reserve hardware registers.
>   Reserved.set(Mips::HWR29);
>   Reserved.set(Mips::HWR29_64);
> 
> Added: llvm/trunk/test/CodeGen/Mips/fp-spill-reload.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/fp-spill-reload.ll?rev=156458&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/Mips/fp-spill-reload.ll (added)
> +++ llvm/trunk/test/CodeGen/Mips/fp-spill-reload.ll Tue May  8 20:38:13 2012
> @@ -0,0 +1,39 @@
> +; RUN: llc -march=mipsel < %s | FileCheck %s
> +; check that $fp is not reserved. 
> +
> +define void @foo0(i32* nocapture %b) nounwind {
> +entry:
> +; CHECK: sw  $fp
> +; CHECK: lw  $fp
> +  %0 = load i32* %b, align 4
> +  %arrayidx.1 = getelementptr inbounds i32* %b, i32 1
> +  %1 = load i32* %arrayidx.1, align 4
> +  %add.1 = add nsw i32 %1, 1
> +  %arrayidx.2 = getelementptr inbounds i32* %b, i32 2
> +  %2 = load i32* %arrayidx.2, align 4
> +  %add.2 = add nsw i32 %2, 2
> +  %arrayidx.3 = getelementptr inbounds i32* %b, i32 3
> +  %3 = load i32* %arrayidx.3, align 4
> +  %add.3 = add nsw i32 %3, 3
> +  %arrayidx.4 = getelementptr inbounds i32* %b, i32 4
> +  %4 = load i32* %arrayidx.4, align 4
> +  %add.4 = add nsw i32 %4, 4
> +  %arrayidx.5 = getelementptr inbounds i32* %b, i32 5
> +  %5 = load i32* %arrayidx.5, align 4
> +  %add.5 = add nsw i32 %5, 5
> +  %arrayidx.6 = getelementptr inbounds i32* %b, i32 6
> +  %6 = load i32* %arrayidx.6, align 4
> +  %add.6 = add nsw i32 %6, 6
> +  %arrayidx.7 = getelementptr inbounds i32* %b, i32 7
> +  %7 = load i32* %arrayidx.7, align 4
> +  %add.7 = add nsw i32 %7, 7
> +  call void @foo2(i32 %0, i32 %add.1, i32 %add.2, i32 %add.3, i32 %add.4, i32 %add.5, i32 %add.6, i32 %add.7) nounwind
> +  call void bitcast (void (...)* @foo1 to void ()*)() nounwind
> +  call void @foo2(i32 %0, i32 %add.1, i32 %add.2, i32 %add.3, i32 %add.4, i32 %add.5, i32 %add.6, i32 %add.7) nounwind
> +  ret void
> +}
> +
> +declare void @foo2(i32, i32, i32, i32, i32, i32, i32, i32)
> +
> +declare void @foo1(...)
> +
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits