[llvm] [GlobalISel][AMDGPU] Do not cache UsesAGPRs from empty functions. (PR #124799)

[David Green via llvm-commits]

https://github.com/davemgreen updated https://github.com/llvm/llvm-project/pull/124799

>From 7e95393f8cca1e7d018d300d97e8cb90bd5864ff Mon Sep 17 00:00:00 2001
From: David Green <david.green at arm.com>
Date: Tue, 28 Jan 2025 17:31:27 +0000
Subject: [PATCH 1/2] [GlobalISel][AMDGPU] Do not cache UsesAGPRs from empty
 functions.

After we fall back from global-isel to SDAG, the verifier gets called, which
calls getReservedRegs which uses SIMachineFunctionInfo::usesAGPRs which caches
the result of UsesAGPRs. Because we have just fallen-back the function is empty
and it incorrectly gets cached to false. This patch makes sure we don't try to
cache incorrect information from a function in an empty state.
---
 llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp    | 6 ++++++
 llvm/test/CodeGen/AMDGPU/llvm.amdgcn.mfma.gfx950.ll | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
index c5efb89d8b2dbc..64026cd237d179 100644
--- a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
@@ -788,6 +788,12 @@ bool SIMachineFunctionInfo::usesAGPRs(const MachineFunction &MF) const {
   if (UsesAGPRs)
     return *UsesAGPRs;
 
+  // Assume we cannot get any useful information about an empty function, but do
+  // not cache the result as we may not have useful information yet (for example
+  // after a Global-ISel fallback).
+  if (MF.empty())
+    return false;
+
   if (!mayNeedAGPRs()) {
     UsesAGPRs = false;
     return false;
diff --git a/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.mfma.gfx950.ll b/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.mfma.gfx950.ll
index d9ee276c3f076e..44cb4e803ffad6 100644
--- a/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.mfma.gfx950.ll
+++ b/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.mfma.gfx950.ll
@@ -1,6 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
 ; RUN: llc -mtriple=amdgcn -mcpu=gfx950 -global-isel=0 < %s | FileCheck -enable-var-scope --check-prefixes=GCN,SDAG %s
-; RUN: llc -mtriple=amdgcn -mcpu=gfx950 -global-isel=1 -global-isel-abort=2 < %s | FileCheck -enable-var-scope --check-prefixes=GCN,GISEL %s
+; RUN: llc -mtriple=amdgcn -mcpu=gfx950 -global-isel=1 -global-isel-abort=2 -verify-machineinstrs < %s | FileCheck -enable-var-scope --check-prefixes=GCN,GISEL %s
 
 declare <4 x float> @llvm.amdgcn.mfma.f32.16x16x32.f16(<8 x half>, <8 x half>, <4 x float>, i32 immarg, i32 immarg, i32 immarg)
 declare <16 x float> @llvm.amdgcn.mfma.f32.32x32x16.f16(<8 x half>, <8 x half>, <16 x float>, i32 immarg, i32 immarg, i32 immarg)

>From 5fcc0c30dc37620a435bd597a9070e5635d10d22 Mon Sep 17 00:00:00 2001
From: David Green <david.green at arm.com>
Date: Wed, 29 Jan 2025 11:29:28 +0000
Subject: [PATCH 2/2] Alternative, where we dont add a verifier pass after
 fallback instead

---
 llvm/lib/CodeGen/TargetPassConfig.cpp            | 6 ++++--
 llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp | 6 ------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/llvm/lib/CodeGen/TargetPassConfig.cpp b/llvm/lib/CodeGen/TargetPassConfig.cpp
index b3046ce83ac5a5..5d9da9df9092a5 100644
--- a/llvm/lib/CodeGen/TargetPassConfig.cpp
+++ b/llvm/lib/CodeGen/TargetPassConfig.cpp
@@ -1039,11 +1039,13 @@ bool TargetPassConfig::addCoreISelPasses() {
 
     if (addGlobalInstructionSelect())
       return true;
+  }
 
-    // Pass to reset the MachineFunction if the ISel failed.
+  // Pass to reset the MachineFunction if the ISel failed. Outside of the above
+  // if so that the verifier is not added to it.
+  if (Selector == SelectorType::GlobalISel)
     addPass(createResetMachineFunctionPass(
         reportDiagnosticWhenGlobalISelFallback(), isGlobalISelAbortEnabled()));
-  }
 
   // Run the SDAG InstSelector, providing a fallback path when we do not want to
   // abort on not-yet-supported input.
diff --git a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
index 64026cd237d179..c5efb89d8b2dbc 100644
--- a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
@@ -788,12 +788,6 @@ bool SIMachineFunctionInfo::usesAGPRs(const MachineFunction &MF) const {
   if (UsesAGPRs)
     return *UsesAGPRs;
 
-  // Assume we cannot get any useful information about an empty function, but do
-  // not cache the result as we may not have useful information yet (for example
-  // after a Global-ISel fallback).
-  if (MF.empty())
-    return false;
-
   if (!mayNeedAGPRs()) {
     UsesAGPRs = false;
     return false;