[PATCH] D107404: [AMDGPU] Avoid assert for saved FP

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec created this revision.
rampitec added a reviewer: arsenm.
Herald added subscribers: foad, kerbowa, hiraditya, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl.
rampitec requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

With spilling into AGPRs enabled we cannot reliably predict
if we need to save FP or not. We may finally spill everything
into AGPRs and never touch stack. In this case we still may
save FP. This is deficiency but not an error, so avoid the
assert.


https://reviews.llvm.org/D107404

Files:
  llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
  llvm/test/CodeGen/AMDGPU/save-fp.ll


Index: llvm/test/CodeGen/AMDGPU/save-fp.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/save-fp.ll
@@ -0,0 +1,29 @@
+; RUN: llc -march=amdgcn -mcpu=gfx908 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX908 %s
+; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX900 %s
+
+define void @foo() {
+bb:
+  ret void
+}
+
+; FIXME: We spill v40 into AGPR, but still save and restore FP
+; which is not needed in this case.
+
+; GCN-LABEL: {{^}}caller:
+
+; GFX900:     s_mov_b32 [[SAVED_FP:s[0-9]+]], s33
+; GFX900:     s_mov_b32 s33, s32
+; GFX908-NOT: s_mov_b32 s33, s32
+; GFX900:     buffer_store_dword
+; GFX908-DAG: s_mov_b32 [[SAVED_FP:s[0-9]+]], s33
+; GFX908-DAG: v_accvgpr_write_b32
+; GCN:        s_swappc_b64
+; GFX900:     buffer_load_dword
+; GFX908:     v_accvgpr_read_b32
+; GCN:        s_mov_b32 s33, [[SAVED_FP]]
+define i64 @caller() {
+bb:
+  call void asm sideeffect "", "~{v40}" ()
+  tail call void @foo()
+  ret i64 0
+}
Index: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
+++ llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
@@ -959,8 +959,11 @@
                      FuncInfo->FramePointerSaveIndex)) &&
          "Needed to save FP but didn't save it anywhere");
 
+  // If we allow spilling to AGPRs we may have saved FP but then spill
+  // everythying into AGPRs instead of the stack.
   assert((HasFP || (!FuncInfo->SGPRForFPSaveRestoreCopy &&
-                    !FuncInfo->FramePointerSaveIndex)) &&
+                    !FuncInfo->FramePointerSaveIndex) ||
+                   EnableSpillVGPRToAGPR) &&
          "Saved FP but didn't need it");
 
   assert((!HasBP || (FuncInfo->SGPRForBPSaveRestoreCopy ||


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107404.363884.patch
Type: text/x-patch
Size: 1871 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210803/7d07bf63/attachment-0001.bin>