[llvm] 11b7ee9 - [AMDGPU] Avoid assert for saved FP

[Stanislav Mekhanoshin via llvm-commits]

Author: Stanislav Mekhanoshin
Date: 2021-08-25T09:50:59-07:00
New Revision: 11b7ee974a69cabb379b12ce62a808594a53ef95

URL: https://github.com/llvm/llvm-project/commit/11b7ee974a69cabb379b12ce62a808594a53ef95
DIFF: https://github.com/llvm/llvm-project/commit/11b7ee974a69cabb379b12ce62a808594a53ef95.diff

LOG: [AMDGPU] Avoid assert for saved FP

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.

Differential Revision: https://reviews.llvm.org/D107404

Added: 
    llvm/test/CodeGen/AMDGPU/save-fp.ll

Modified: 
    llvm/lib/Target/AMDGPU/SIFrameLowering.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
index fe0132bb8e80..a567e89ac3b7 100644
--- a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
@@ -959,8 +959,11 @@ void SIFrameLowering::emitPrologue(MachineFunction &MF,
                      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
+  // everything 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 ||

diff  --git a/llvm/test/CodeGen/AMDGPU/save-fp.ll b/llvm/test/CodeGen/AMDGPU/save-fp.ll
new file mode 100644
index 000000000000..6bed1dbf901e
--- /dev/null
+++ b/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
+}