[AMDGPU] Fix AGPR_32 reg assign for mfma scale ops #168964
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In MFMA rewrite pass, prevent AGPR_32 reg class assignment for scale operands, not permitted by instruction format.
Member
|
@llvm/pr-subscribers-backend-amdgpu Author: None (hjagasiaAMD) ChangesIn MFMA rewrite pass, prevent AGPR_32 reg class assignment for scale operands, not permitted by instruction format. Full diff: https://github.com/llvm/llvm-project/pull/168964.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPURewriteAGPRCopyMFMA.cpp b/llvm/lib/Target/AMDGPU/AMDGPURewriteAGPRCopyMFMA.cpp
index 89c16dadb4b41..b5e3187289160 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPURewriteAGPRCopyMFMA.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPURewriteAGPRCopyMFMA.cpp
@@ -302,6 +302,10 @@ bool AMDGPURewriteAGPRCopyMFMAImpl::attemptReassignmentsToAGPR(
const TargetRegisterClass *EquivalentAGPRRegClass =
TRI.getEquivalentAGPRClass(MRI.getRegClass(InterferingReg));
+ // Do not reassign scale operands
+ if (EquivalentAGPRRegClass == &AMDGPU::AGPR_32RegClass)
+ return false;
+
MCPhysReg Assignable = AMDGPU::NoRegister;
if (EquivalentAGPRRegClass->contains(PrefPhysReg) &&
LRM.checkInterference(ReassignLI, PrefPhysReg) ==
diff --git a/llvm/test/CodeGen/AMDGPU/rewrite-vgpr-mfma-scale-to-agpr.mir b/llvm/test/CodeGen/AMDGPU/rewrite-vgpr-mfma-scale-to-agpr.mir
index ab56c9982753f..12be806960b67 100644
--- a/llvm/test/CodeGen/AMDGPU/rewrite-vgpr-mfma-scale-to-agpr.mir
+++ b/llvm/test/CodeGen/AMDGPU/rewrite-vgpr-mfma-scale-to-agpr.mir
@@ -1,6 +1,6 @@
-# RUN: not llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx950 -run-pass=greedy,amdgpu-rewrite-agpr-copy-mfma -verify-machineinstrs -o - %s 2>&1 | FileCheck %s
-# CHECK: Illegal virtual register for instruction
-# CHECK: Expected a VGPR_32 register, but got a AGPR_32 register
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx950 -run-pass=greedy,amdgpu-rewrite-agpr-copy-mfma -verify-machineinstrs -o - %s 2>&1 | FileCheck %s
+# CHECK-NOT: Illegal virtual register for instruction
+# CHECK-NOT: Expected a VGPR_32 register, but got a AGPR_32 register
# Test for issue in amdgpu-rewrite-agpr-copy-mfma, which reassigns scale operand
# in vgpr_32 register to agpr_32, not permitted by instruction format.
|
arsenm
reviewed
Nov 20, 2025
| # CHECK: Illegal virtual register for instruction | ||
| # CHECK: Expected a VGPR_32 register, but got a AGPR_32 register | ||
| # RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx950 -run-pass=greedy,amdgpu-rewrite-agpr-copy-mfma -verify-machineinstrs -o - %s 2>&1 | FileCheck %s | ||
| # CHECK-NOT: Illegal virtual register for instruction |
Contributor
There was a problem hiding this comment.
-NOT checks are close to useless, especially for checking error messages. Generate checks for the actual output
| TRI.getEquivalentAGPRClass(MRI.getRegClass(InterferingReg)); | ||
|
|
||
| // Do not reassign scale operands | ||
| if (EquivalentAGPRRegClass == &AMDGPU::AGPR_32RegClass) |
Contributor
There was a problem hiding this comment.
This doesn't seem like the right condition. It just happens the scale operands are the only 32-bit input case. This should more directly check the operand constraint instead of checking a hardcodes class equality
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
arsenm
reviewed
Dec 1, 2025
| if (isRewriteCandidate(*MI)) { | ||
|
|
||
| int AGPROp = AMDGPU::getMFMASrcCVDstAGPROp(MI->getOpcode()); | ||
| MachineInstrBuilder TmpMIB = |
Contributor
There was a problem hiding this comment.
Definitely should not be creating temporary instructions
| unsigned OpNo = &MO - &MI->getOperand(0); | ||
| const TargetRegisterClass *EquivalentAGPRRegClass = | ||
| TRI.getEquivalentAGPRClass(MRI.getRegClass(Reg)); | ||
| const TargetRegisterClass *Allowed = TmpMI->getRegClassConstraintEffect( |
Contributor
There was a problem hiding this comment.
You want TargetInstrInfo::getRegClass to get the static constraint of the known operand (alternatively, you could check that the use is one of the known src0/src1 operands and not the _scale name)
arsenm
reviewed
Dec 2, 2025
Co-authored-by: Matt Arsenault <[email protected]>
Co-authored-by: Matt Arsenault <[email protected]>
arsenm
reviewed
Dec 2, 2025
Co-authored-by: Matt Arsenault <[email protected]>
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
arsenm
approved these changes
Dec 2, 2025
kcloudy0717
pushed a commit
to kcloudy0717/llvm-project
that referenced
this pull request
Dec 4, 2025
In MFMA rewrite pass, prevent AGPR_32 reg class assignment for scale operands, not permitted by instruction format. --------- Co-authored-by: Matt Arsenault <[email protected]>
honeygoyal
pushed a commit
to honeygoyal/llvm-project
that referenced
this pull request
Dec 9, 2025
In MFMA rewrite pass, prevent AGPR_32 reg class assignment for scale operands, not permitted by instruction format. --------- Co-authored-by: Matt Arsenault <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In MFMA rewrite pass, prevent AGPR_32 reg class assignment for scale operands, not permitted by instruction format.