AMDGPU: Make frame register caller preserved

[arsenm]

Summary

Using SplitCSR for the frame register was very broken. Often
the copies in the prolog and epilog were optimized out, in addition
to them being inserted after the true prolog where the FP
was clobbered.

I have a hacky solution which works that continues to use
split CSR, but for now this is simpler and will get to working
programs.

[rampitec]

Please add a comment to the source why it was needed.

[arsenm]

Add comment

[rampitec]

LGTM

[arsenm]

r313274

lib/Target/AMDGPU/AMDGPURegisterInfo.cpp

default: {
     static const MCPhysReg NoCalleeSavedReg = AMDGPU::NoRegister;
     return &NoCalleeSavedReg;
   }
   }
 }
 
 const MCPhysReg *
 SIRegisterInfo::getCalleeSavedRegsViaCopy(const MachineFunction *MF) const {
-  // FIXME
-  static MCPhysReg Regs[2];
-
-  const SIMachineFunctionInfo *MFI = MF->getInfo<SIMachineFunctionInfo>();
-  assert(!MFI->isEntryFunction());
-
-  Regs[0] = MFI->getFrameOffsetReg();
-  Regs[1] = AMDGPU::NoRegister;
-
-  return Regs;
+  return nullptr;
 }
 
 const uint32_t *SIRegisterInfo::getCallPreservedMask(const MachineFunction &MF,
                                                      CallingConv::ID CC) const {
   switch (CC) {
   case CallingConv::C:
   case CallingConv::Fast:
   case CallingConv::Cold:

lib/Target/AMDGPU/SIISelLowering.cpp

SDValue SITargetLowering::LowerCall(CallLoweringInfo &CLI,
   // Stores to callee stack arguments will be placed in FixedStackSlots offset
   // by this amount for a tail call. In a sibling call it must be 0 because the
   // caller will deallocate the entire stack and the callee still expects its
   // arguments to begin at SP+0. Completely unused for non-tail calls.
   int32_t FPDiff = 0;
   MachineFrameInfo &MFI = MF.getFrameInfo();
   SmallVector<std::pair<unsigned, SDValue>, 8> RegsToPass;
 
+  SDValue CallerSavedFP;
+
   // Adjust the stack pointer for the new arguments...
   // These operations are automatically eliminated by the prolog/epilog pass
   if (!IsSibCall) {
     Chain = DAG.getCALLSEQ_START(Chain, NumBytes, 0, DL);
 
     unsigned OffsetReg = Info->getScratchWaveOffsetReg();
 
     // In the HSA case, this should be an identity copy.
     SDValue ScratchRSrcReg
       = DAG.getCopyFromReg(Chain, DL, Info->getScratchRSrcReg(), MVT::v4i32);
     RegsToPass.emplace_back(AMDGPU::SGPR0_SGPR1_SGPR2_SGPR3, ScratchRSrcReg);
 
     // TODO: Don't hardcode these registers and get from the callee function.
     SDValue ScratchWaveOffsetReg
       = DAG.getCopyFromReg(Chain, DL, OffsetReg, MVT::i32);
     RegsToPass.emplace_back(AMDGPU::SGPR4, ScratchWaveOffsetReg);
+
+    if (!Info->isEntryFunction()) {
+      // Avoid clobbering this function's FP value. In the current convention
+      // callee will overwrite this, so do save/restore around the call site.
+      CallerSavedFP = DAG.getCopyFromReg(Chain, DL,
+                                         Info->getFrameOffsetReg(), MVT::i32);
+    }
   }
 
   // Stack pointer relative accesses are done by changing the offset SGPR. This
   // is just the VGPR offset component.
   SDValue StackPtr = DAG.getConstant(CalleeUsableStackOffset, DL, MVT::i32);
 
   SmallVector<SDValue, 8> MemOpChains;
   MVT PtrVT = MVT::i32;
if (IsTailCall) {
     return DAG.getNode(AMDGPUISD::TC_RETURN, DL, NodeTys, Ops);
   }
 
   // Returns a chain and a flag for retval copy to use.
   SDValue Call = DAG.getNode(AMDGPUISD::CALL, DL, NodeTys, Ops);
   Chain = Call.getValue(0);
   InFlag = Call.getValue(1);
 
+  if (CallerSavedFP) {
+    SDValue FPReg = DAG.getRegister(Info->getFrameOffsetReg(), MVT::i32);
+    Chain = DAG.getCopyToReg(Chain, DL, FPReg, CallerSavedFP, InFlag);
+    InFlag = Chain.getValue(1);
+  }
+
   uint64_t CalleePopBytes = 0;
   Chain = DAG.getCALLSEQ_END(Chain, DAG.getTargetConstant(NumBytes, DL, MVT::i32),
                              DAG.getTargetConstant(CalleePopBytes, DL, MVT::i32),
                              InFlag, DL);
   if (!Ins.empty())
     InFlag = Chain.getValue(1);
 
   // Handle result values, copying them out of physregs into vregs that we

test/CodeGen/AMDGPU/call-preserved-registers.ll

 ; GCN-LABEL: {{^}}test_func_call_external_void_func_void_clobber_s30_s31_call_external_void_func_void:
 ; GCN: v_writelane_b32 v32, s33, 0
 ; GCN: v_writelane_b32 v32, s34, 1
 ; GCN: v_writelane_b32 v32, s35, 2
 ; GCN: v_writelane_b32 v32, s36, 3
 ; GCN: v_writelane_b32 v32, s37, 4
 
 ; GCN: s_mov_b32 s33, s5
-; GCN: s_swappc_b64
+; GCN-NEXT: s_swappc_b64
+; GCN-NEXT: s_mov_b32 s5, s33
 ; GCN-NEXT: ;;#ASMSTART
 ; GCN-NEXT: ;;#ASMEND
+; GCN-NEXT: s_mov_b32 s33, s5
 ; GCN-NEXT: s_swappc_b64
-; GCN: s_mov_b32 s5, s33
+; GCN-NEXT: s_mov_b32 s5, s33
 ; GCN: v_readlane_b32 s37, v32, 4
 ; GCN: v_readlane_b32 s36, v32, 3
 ; GCN: v_readlane_b32 s35, v32, 2
 ; GCN: v_readlane_b32 s34, v32, 1
 ; GCN: v_readlane_b32 s33, v32, 0
 ; GCN: s_setpc_b64
 define void @test_func_call_external_void_func_void_clobber_s30_s31_call_external_void_func_void() #0 {
   call void @external_void_func_void()
   call void asm sideeffect "", ""() #0
   call void @external_void_func_void()
   ret void
 }
 
+; FIXME: Avoid extra restore of FP in between calls.
+; GCN-LABEL: {{^}}test_func_call_external_void_funcx2:
+; GCN: s_mov_b32 s33, s5
+; GCN-NEXT: s_swappc_b64
+; GCN-NEXT: s_mov_b32 s5, s33
+; GCN-NEXT: s_mov_b32 s33, s5
+; GCN-NEXT: s_swappc_b64
+; GCN-NEXT: s_mov_b32 s5, s33
+define void @test_func_call_external_void_funcx2() #0 {
+  call void @external_void_func_void()
+  call void @external_void_func_void()
+  ret void
+}
+
 ; GCN-LABEL: {{^}}void_func_void_clobber_s30_s31:
 ; GCN: s_waitcnt
 ; GCN-NEXT: s_mov_b64 [[SAVEPC:s\[[0-9]+:[0-9]+\]]], s[30:31]
 ; GCN-NEXT: #ASMSTART
 ; GCN: ; clobber
 ; GCN-NEXT: #ASMEND
 ; GCN-NEXT: s_mov_b64 s[30:31], [[SAVEPC]]
 ; GCN-NEXT: s_setpc_b64 s[30:31]

test/CodeGen/AMDGPU/ipra.ll

 define void @func_call_tail_call() #1 {
   %vgpr = load volatile i32, i32 addrspace(1)* undef
   tail call void @func()
   store volatile i32 %vgpr, i32 addrspace(1)* undef
   tail call void @func()
   ret void
 }
 
+define void @void_func_void() noinline {
+  ret void
+}
+
+; Make sure we don't get save/restore of FP between calls.
+; GCN-LABEL: {{^}}test_funcx2:
+; GCN-NOT: s5
+; GCN-NOT: s32
+define void @test_funcx2() #0 {
+  call void @void_func_void()
+  call void @void_func_void()
+  ret void
+}
+
 attributes #0 = { nounwind }
 attributes #1 = { nounwind noinline }

test/CodeGen/AMDGPU/sibling-call.ll

 ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=fiji -mattr=-flat-for-global -amdgpu-sroa=0 -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,VI,MESA %s
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=hawaii -amdgpu-sroa=0 -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,CI,MESA %s
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -mattr=-flat-for-global -amdgpu-sroa=0 -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX9,VI,MESA %s
 
 ; GCN-LABEL: {{^}}i32_fastcc_i32_i32:
 ; GCN: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GCN-NEXT: v_add_i32_e32 v0, vcc, v1, v0
 ; GCN-NEXT: s_setpc_b64
 define fastcc i32 @i32_fastcc_i32_i32(i32 %arg0, i32 %arg1) #1 {
   %add0 = add i32 %arg0, %arg1
   ret i32 %add0
 }
 
 ; GCN-LABEL: {{^}}i32_fastcc_i32_i32_stack_object:
 ; GCN: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GCN: s_mov_b32 s5, s32
 ; GCN: v_add_i32_e32 v0, vcc, v1, v
+; GCN: s_mov_b32 s5, s32
 ; GCN: buffer_store_dword v{{[0-9]+}}, off, s[0:3], s5 offset:24
 ; GCN: s_waitcnt vmcnt(0)
 ; GCN: s_setpc_b64
 ; GCN: ; ScratchSize: 68
 define fastcc i32 @i32_fastcc_i32_i32_stack_object(i32 %arg0, i32 %arg1) #1 {
   %alloca = alloca [16 x i32], align 4
   %gep = getelementptr inbounds [16 x i32], [16 x i32]* %alloca, i32 0, i32 5
   store volatile i32 9, i32* %gep