AMDGPU: Delete seemingly dead s_fmaak_f32/s_fmamk_f32 folding code

[arsenm]

No tests fail with this. I'm not sure I understand the comment,
there can't be any folding into an operand that had to already
be a constant. I tried different combinations of immediates to these
instructions but never hit the condition.

[arsenm]

• AMDGPU: Handle folding vector splats of inline split f64 inline immediates #140878
• AMDGPU: Fix tracking subreg defs when folding through reg_sequence #140608
• AMDGPU: Add baseline tests for #139317 #140607
• AMDGPU: Remove redundant operand folding checks #140587
• AMDGPU: Delete seemingly dead s_fmaak_f32/s_fmamk_f32 folding code #140580 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

No tests fail with this. I'm not sure I understand the comment,
there can't be any folding into an operand that had to already
be a constant. I tried different combinations of immediates to these
instructions but never hit the condition.

---

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

1 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/SIFoldOperands.cpp (-11)

diff --git a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
index 92937e33fd500..d94c2d8b03dff 100644
--- a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
@@ -767,17 +767,6 @@ bool SIFoldOperandsImpl::tryAddToFoldList(
     return true;
   }
 
-  // Inlineable constant might have been folded into Imm operand of fmaak or
-  // fmamk and we are trying to fold a non-inlinable constant.
-  if ((Opc == AMDGPU::S_FMAAK_F32 || Opc == AMDGPU::S_FMAMK_F32) &&
-      !OpToFold->isReg() && !TII->isInlineConstant(*OpToFold)) {
-    unsigned ImmIdx = Opc == AMDGPU::S_FMAAK_F32 ? 3 : 2;
-    MachineOperand &OpImm = MI->getOperand(ImmIdx);
-    if (!OpImm.isReg() &&
-        TII->isInlineConstant(*MI, MI->getOperand(OpNo), OpImm))
-      return tryToFoldAsFMAAKorMK();
-  }
-
   // Special case for s_fmac_f32 if we are trying to fold into Src0 or Src1.
   // By changing into fmamk we can untie Src2.
   // If folding for Src0 happens first and it is identical operand to Src1 we

[Sisyph]

My interpretation of the comment is: some immediates are inlinable and some are not. So if we had an inlinable constant in the mandatory literal operand slot of the instruction, we want to commute the operands so the one that is inlinable is in the inlinable slot.

As to whether it's for some reason dead code, I can't say.

[jayfoad]

No tests fail with this.

That's because the three cases that were supposed to exercise this code in test/CodeGen/AMDGPU/fold-operands-scalar-fmac.mir are exactly the ones that were previously pessimized by #127626.

[mbrkusanin]

Cases that this was supposed to cover can no longer happen in this part of the code due to #127563 and #127626.

[mbrkusanin]

Cases that this was supposed to cover can no longer happen in this part of the code due to #127563 and #127626.

We used to prefer s_fmamk and s_fmamk over s_fmac with inlinable constants because they had untied src2 from dst.
This code was working with cases where inlinable constant was in the literal slot of fmamk or fmaak and we wanted to fold another noninlinable one.

Either way due to other changes this can no longer be used in the same way.

[arsenm]

Merge activity

• May 29, 5:28 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• May 29, 5:31 PM UTC: Graphite rebased this pull request as part of a merge.
• May 29, 5:33 PM UTC: Graphite rebased this pull request as part of a merge.
• May 29, 5:36 PM UTC: @arsenm merged this pull request with Graphite.