[llvm] [AMDGPU] Simplify immediate checking and cleanup in omod folding. NFC (PR #218217)
Changpeng Fang via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 23 00:55:13 PDT 2026
https://github.com/changpeng created https://github.com/llvm/llvm-project/pull/218217
The key changes are:
1. Replace std::pair() with brace initialization {}
2. Simplify immediate operand detection: Instead of checking both Src0 and Src1 to find which one is immediate, we now use getImmOrMaterializedImm on Src1 directly, since after canonicalization the immediate is always in Src1.
3. Extract the register into a variable (OModSrcReg) for better readability
The new tests cover cases where the immediate appears first in the IR (e.g., 4.0 * %add), which after DAG canonicalization becomes (%add * 4.0), ensuring the immediate is always in the second operand position. These tests verify that omod folding works correctly after canonicalization.
No functional change intended.
>From af3da68f5d3b02dcd6a8c008e350fcea51deb8b9 Mon Sep 17 00:00:00 2001
From: Changpeng Fang <changpeng.fang at amd.com>
Date: Sun, 23 Aug 2026 00:38:41 -0700
Subject: [PATCH] [AMDGPU] Simplify immediate checking and cleanup in omod
folding. NFC
The key changes are:
1. Replace std::pair() with brace initialization {}
2. Simplify immediate operand detection: Instead of checking both Src0
and Src1 to find which one is immediate, we now use
getImmOrMaterializedImm on Src1 directly, since after canonicalization
the immediate is always in Src1.
3. Extract the register into a variable (OModSrcReg) for better readability
The new tests cover cases where the immediate appears first in the IR
(e.g., 4.0 * %add), which after DAG canonicalization becomes (%add * 4.0),
ensuring the immediate is always in the second operand position. These
tests verify that omod folding works correctly after canonicalization.
No functional change intended.
---
llvm/lib/Target/AMDGPU/SIFoldOperands.cpp | 37 ++++----
llvm/test/CodeGen/AMDGPU/omod.ll | 106 ++++++++++++++++++++++
2 files changed, 123 insertions(+), 20 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
index ad7917a25963f..584efa001f2a5 100644
--- a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
@@ -2441,30 +2441,26 @@ SIFoldOperandsImpl::isOMod(const MachineInstr &MI) const {
MFI->getMode().FP64FP16Denormals.Output !=
DenormalMode::PreserveSign) ||
MI.mayRaiseFPException())
- return std::pair(nullptr, SIOutMods::NONE);
+ return {nullptr, SIOutMods::NONE};
- const MachineOperand *RegOp = nullptr;
- const MachineOperand *ImmOp = nullptr;
const MachineOperand *Src0 = TII->getNamedOperand(MI, AMDGPU::OpName::src0);
const MachineOperand *Src1 = TII->getNamedOperand(MI, AMDGPU::OpName::src1);
- if (Src0->isImm()) {
- ImmOp = Src0;
- RegOp = Src1;
- } else if (Src1->isImm()) {
- ImmOp = Src1;
- RegOp = Src0;
- } else
- return std::pair(nullptr, SIOutMods::NONE);
- int OMod = getOModValue(Op, ImmOp->getImm());
+ // If there is an immediate operand, it must be Src1
+ std::optional<int64_t> Src1Imm =
+ TII->getImmOrMaterializedImm(const_cast<MachineOperand &>(*Src1));
+ if (!Src1Imm)
+ return {nullptr, SIOutMods::NONE};
+
+ int OMod = getOModValue(Op, *Src1Imm);
if (OMod == SIOutMods::NONE ||
TII->hasModifiersSet(MI, AMDGPU::OpName::src0_modifiers) ||
TII->hasModifiersSet(MI, AMDGPU::OpName::src1_modifiers) ||
TII->hasModifiersSet(MI, AMDGPU::OpName::omod) ||
TII->hasModifiersSet(MI, AMDGPU::OpName::clamp))
- return std::pair(nullptr, SIOutMods::NONE);
+ return {nullptr, SIOutMods::NONE};
- return std::pair(RegOp, OMod);
+ return {Src0, OMod};
}
case AMDGPU::V_ADD_F64_e64:
case AMDGPU::V_ADD_F64_pseudo_e64:
@@ -2479,7 +2475,7 @@ SIFoldOperandsImpl::isOMod(const MachineInstr &MI) const {
Op == AMDGPU::V_ADD_F16_e64 || Op == AMDGPU::V_ADD_F16_t16_e64 ||
Op == AMDGPU::V_ADD_F16_fake16_e64) &&
MFI->getMode().FP64FP16Denormals.Output != DenormalMode::PreserveSign))
- return std::pair(nullptr, SIOutMods::NONE);
+ return {nullptr, SIOutMods::NONE};
// Look through the DAGCombiner canonicalization fmul x, 2 -> fadd x, x
const MachineOperand *Src0 = TII->getNamedOperand(MI, AMDGPU::OpName::src0);
@@ -2491,12 +2487,12 @@ SIFoldOperandsImpl::isOMod(const MachineInstr &MI) const {
!TII->hasModifiersSet(MI, AMDGPU::OpName::src1_modifiers) &&
!TII->hasModifiersSet(MI, AMDGPU::OpName::clamp) &&
!TII->hasModifiersSet(MI, AMDGPU::OpName::omod))
- return std::pair(Src0, SIOutMods::MUL2);
+ return {Src0, SIOutMods::MUL2};
- return std::pair(nullptr, SIOutMods::NONE);
+ return {nullptr, SIOutMods::NONE};
}
default:
- return std::pair(nullptr, SIOutMods::NONE);
+ return {nullptr, SIOutMods::NONE};
}
}
@@ -2526,10 +2522,11 @@ bool SIFoldOperandsImpl::tryFoldOMod(MachineInstr &MI) {
LLVM_DEBUG(dbgs() << "Folding omod " << MI << " into " << *Def);
DefOMod->setImm(OMod);
- MRI->replaceRegWith(MI.getOperand(0).getReg(), Def->getOperand(0).getReg());
+ Register OModSrcReg = Def->getOperand(0).getReg();
+ MRI->replaceRegWith(MI.getOperand(0).getReg(), OModSrcReg);
// Kill flags can be wrong if we replaced a def inside a loop with a def
// outside the loop.
- MRI->clearKillFlags(Def->getOperand(0).getReg());
+ MRI->clearKillFlags(OModSrcReg);
MI.eraseFromParent();
// Use of output modifiers forces VOP3 encoding for a VOP2 mac/fmac
diff --git a/llvm/test/CodeGen/AMDGPU/omod.ll b/llvm/test/CodeGen/AMDGPU/omod.ll
index e2121c99ceac7..b8be673504feb 100644
--- a/llvm/test/CodeGen/AMDGPU/omod.ll
+++ b/llvm/test/CodeGen/AMDGPU/omod.ll
@@ -546,6 +546,32 @@ define amdgpu_ps void @v_omod_mul4_f32(float %a) #0 {
ret void
}
+define amdgpu_ps void @v_omod_mul4_f32_imm_first(float %a) #0 {
+; SI-LABEL: v_omod_mul4_f32_imm_first:
+; SI: ; %bb.0:
+; SI-NEXT: v_add_f32_e64 v0, v0, 1.0 mul:4
+; SI-NEXT: s_mov_b32 s3, 0xf000
+; SI-NEXT: s_mov_b32 s2, -1
+; SI-NEXT: buffer_store_dword v0, off, s[0:3], 0
+; SI-NEXT: s_endpgm
+;
+; VI-LABEL: v_omod_mul4_f32_imm_first:
+; VI: ; %bb.0:
+; VI-NEXT: v_add_f32_e64 v0, v0, 1.0 mul:4
+; VI-NEXT: flat_store_dword v[0:1], v0
+; VI-NEXT: s_endpgm
+;
+; GFX11PLUS-LABEL: v_omod_mul4_f32_imm_first:
+; GFX11PLUS: ; %bb.0:
+; GFX11PLUS-NEXT: v_add_f32_e64 v0, v0, 1.0 mul:4
+; GFX11PLUS-NEXT: global_store_b32 v[0:1], v0, off
+; GFX11PLUS-NEXT: s_endpgm
+ %add = fadd float %a, 1.0
+ %div2 = fmul nsz float 4.0, %add
+ store float %div2, ptr addrspace(1) poison
+ ret void
+}
+
define amdgpu_ps void @v_omod_mul4_f64(double %a) #5 {
; SI-LABEL: v_omod_mul4_f64:
; SI: ; %bb.0:
@@ -578,6 +604,38 @@ define amdgpu_ps void @v_omod_mul4_f64(double %a) #5 {
ret void
}
+define amdgpu_ps void @v_omod_mul4_f64_imm_first(double %a) #5 {
+; SI-LABEL: v_omod_mul4_f64_imm_first:
+; SI: ; %bb.0:
+; SI-NEXT: v_add_f64 v[0:1], v[0:1], 1.0 mul:4
+; SI-NEXT: s_mov_b32 s3, 0xf000
+; SI-NEXT: s_mov_b32 s2, -1
+; SI-NEXT: buffer_store_dwordx2 v[0:1], off, s[0:3], 0
+; SI-NEXT: s_endpgm
+;
+; VI-LABEL: v_omod_mul4_f64_imm_first:
+; VI: ; %bb.0:
+; VI-NEXT: v_add_f64 v[0:1], v[0:1], 1.0 mul:4
+; VI-NEXT: flat_store_dwordx2 v[0:1], v[0:1]
+; VI-NEXT: s_endpgm
+;
+; GFX11-LABEL: v_omod_mul4_f64_imm_first:
+; GFX11: ; %bb.0:
+; GFX11-NEXT: v_add_f64 v[0:1], v[0:1], 1.0 mul:4
+; GFX11-NEXT: global_store_b64 v[0:1], v[0:1], off
+; GFX11-NEXT: s_endpgm
+;
+; GFX12-LABEL: v_omod_mul4_f64_imm_first:
+; GFX12: ; %bb.0:
+; GFX12-NEXT: v_add_f64_e64 v[0:1], v[0:1], 1.0 mul:4
+; GFX12-NEXT: global_store_b64 v[0:1], v[0:1], off
+; GFX12-NEXT: s_endpgm
+ %add = fadd nsz double %a, 1.0
+ %div2 = fmul nsz double 4.0, %add
+ store double %div2, ptr addrspace(1) poison
+ ret void
+}
+
define amdgpu_ps void @v_omod_mul4_multi_use_f32(float %a) #0 {
; SI-LABEL: v_omod_mul4_multi_use_f32:
; SI: ; %bb.0:
@@ -1243,6 +1301,54 @@ define amdgpu_ps void @v_omod_div2_f16_no_denormals(half %a) #3 {
ret void
}
+define amdgpu_ps void @v_omod_div2_f16_no_denormals_imm_first(half %a) #3 {
+; SI-LABEL: v_omod_div2_f16_no_denormals_imm_first:
+; SI: ; %bb.0:
+; SI-NEXT: v_cvt_f32_f16_e32 v0, v0
+; SI-NEXT: s_mov_b32 s3, 0xf000
+; SI-NEXT: s_mov_b32 s2, -1
+; SI-NEXT: v_add_f32_e32 v0, 1.0, v0
+; SI-NEXT: v_cvt_f16_f32_e32 v0, v0
+; SI-NEXT: v_cvt_f32_f16_e64 v0, v0 div:2
+; SI-NEXT: v_cvt_f16_f32_e32 v0, v0
+; SI-NEXT: buffer_store_short v0, off, s[0:3], 0
+; SI-NEXT: s_endpgm
+;
+; VI-LABEL: v_omod_div2_f16_no_denormals_imm_first:
+; VI: ; %bb.0:
+; VI-NEXT: v_add_f16_e64 v0, v0, 1.0 div:2
+; VI-NEXT: flat_store_short v[0:1], v0
+; VI-NEXT: s_endpgm
+;
+; GFX11-TRUE16-LABEL: v_omod_div2_f16_no_denormals_imm_first:
+; GFX11-TRUE16: ; %bb.0:
+; GFX11-TRUE16-NEXT: v_add_f16_e64 v0.l, v0.l, 1.0 div:2
+; GFX11-TRUE16-NEXT: global_store_b16 v[0:1], v0, off
+; GFX11-TRUE16-NEXT: s_endpgm
+;
+; GFX11-FAKE16-LABEL: v_omod_div2_f16_no_denormals_imm_first:
+; GFX11-FAKE16: ; %bb.0:
+; GFX11-FAKE16-NEXT: v_add_f16_e64 v0, v0, 1.0 div:2
+; GFX11-FAKE16-NEXT: global_store_b16 v[0:1], v0, off
+; GFX11-FAKE16-NEXT: s_endpgm
+;
+; GFX12-TRUE16-LABEL: v_omod_div2_f16_no_denormals_imm_first:
+; GFX12-TRUE16: ; %bb.0:
+; GFX12-TRUE16-NEXT: v_add_f16_e64 v0.l, v0.l, 1.0 div:2
+; GFX12-TRUE16-NEXT: global_store_b16 v[0:1], v0, off
+; GFX12-TRUE16-NEXT: s_endpgm
+;
+; GFX12-FAKE16-LABEL: v_omod_div2_f16_no_denormals_imm_first:
+; GFX12-FAKE16: ; %bb.0:
+; GFX12-FAKE16-NEXT: v_add_f16_e64 v0, v0, 1.0 div:2
+; GFX12-FAKE16-NEXT: global_store_b16 v[0:1], v0, off
+; GFX12-FAKE16-NEXT: s_endpgm
+ %add = fadd half %a, 1.0
+ %div2 = fmul nsz half 0.5, %add
+ store half %div2, ptr addrspace(1) poison
+ ret void
+}
+
define amdgpu_ps void @v_omod_mac_to_mad(float %b, float %a) #0 {
; SI-LABEL: v_omod_mac_to_mad:
; SI: ; %bb.0:
More information about the llvm-commits
mailing list