[GlobalISel] Do not run verifier after ResetMachineFunctionPass

[davemgreen]

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 run the verifier whilst the function is empty.

[llvmbot]

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-amdgpu

Author: David Green (davemgreen)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/124799.diff

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp (+6)
• (modified) llvm/test/CodeGen/AMDGPU/llvm.amdgcn.mfma.gfx950.ll (+1-1)

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)

[arsenm]

I would hope this could not reach here

[davemgreen]

It comes from: After we fall back from global-isel to SDAG, MachineVerifier::visitMachineFunctionBefore gets called, which calls MRI->getReservedRegs which uses SIMachineFunctionInfo::usesAGPRs which caches the result of UsesAGPRs.
My original patch prevented the call from MachineVerifier to getReservedRegs if the function was empty, but that felt a bit odd. We are not allowed to call getReservedRegs on certain functions at some points? I can get the Machine Verifier not to run at that point in the pipeline by changing how the global isel passes are added. It as far as I understand the verifier after ResetMachineFunctionPass that causes the problems:

diff --git a/llvm/lib/CodeGen/TargetPassConfig.cpp b/llvm/lib/CodeGen/TargetPassConfig.cpp
index 847a1aef39c5..cf407b507303 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.

[arsenm]

Title needs update

[davemgreen]

/cherry-pick 66e0498

[llvmbot]

/pull-request #124917