Skip to content

[AMDGPU] Fix AGPR_32 reg assign for mfma scale ops #168964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ronlieb merged 8 commits into from
Dec 2, 2025
+17 −8
Merged

[AMDGPU] Fix AGPR_32 reg assign for mfma scale ops #168964

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1c2072f
[AMDGPU] Fix AGPR_32 reg assign for mfma scale ops
hjagasiaAMD Nov 20, 2025
d9405f2
Check operand constraints and update mir checks.
hjagasiaAMD Nov 26, 2025
92cc4c8
Merge branch 'main' into agpr-copy-mfma
hjagasiaAMD Dec 1, 2025
08fc310
Get the static constraint of the known operand.
hjagasiaAMD Dec 2, 2025
aff0f88
Update llvm/lib/Target/AMDGPU/AMDGPURewriteAGPRCopyMFMA.cpp
hjagasiaAMD Dec 2, 2025
1fc74ea
Update llvm/lib/Target/AMDGPU/AMDGPURewriteAGPRCopyMFMA.cpp
hjagasiaAMD Dec 2, 2025
ef61605
Update llvm/test/CodeGen/AMDGPU/rewrite-vgpr-mfma-scale-to-agpr.mir
hjagasiaAMD Dec 2, 2025
a55eace
Format
hjagasiaAMD Dec 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions llvm/lib/Target/AMDGPU/AMDGPURewriteAGPRCopyMFMA.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

@arsenm arsenm Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

return false;

MCPhysReg Assignable = AMDGPU::NoRegister;
if (EquivalentAGPRRegClass->contains(PrefPhysReg) &&
LRM.checkInterference(ReassignLI, PrefPhysReg) ==
Expand Down
6 changes: 3 additions & 3 deletions llvm/test/CodeGen/AMDGPU/rewrite-vgpr-mfma-scale-to-agpr.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

@arsenm arsenm Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-NOT checks are close to useless, especially for checking error messages. Generate checks for the actual output

# 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.
Expand Down