From 7e95393f8cca1e7d018d300d97e8cb90bd5864ff Mon Sep 17 00:00:00 2001 From: David Green 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 c5efb89d8b2db..64026cd237d17 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 d9ee276c3f076..44cb4e803ffad 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 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 b3046ce83ac5a..5d9da9df9092a 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 64026cd237d17..c5efb89d8b2db 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;