Skip to content

Conversation

@AbhayKanhere
Copy link
Contributor

After PR:#151421 merged following fails in SIFoldOperands showed up.

LLVM :: CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.mfma.gfx90a.ll
LLVM :: CodeGen/AMDGPU/llvm.amdgcn.mfma.gfx90a.ll
LLVM :: CodeGen/AMDGPU/llvm.amdgcn.mfma.ll
LLVM :: CodeGen/AMDGPU/mfma-loop.ll
LLVM :: CodeGen/AMDGPU/rewrite-vgpr-mfma-to-agpr.ll

In Folding code, if folded operand is register ensure earlyClobber is set.

After PR:llvm#151421 merged
following fails in SIFoldOperands showed up.

LLVM :: CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.mfma.gfx90a.ll
LLVM :: CodeGen/AMDGPU/llvm.amdgcn.mfma.gfx90a.ll
LLVM :: CodeGen/AMDGPU/llvm.amdgcn.mfma.ll
LLVM :: CodeGen/AMDGPU/mfma-loop.ll
LLVM :: CodeGen/AMDGPU/rewrite-vgpr-mfma-to-agpr.ll

In Folding code, if folded operand is register ensure earlyClobber
is set.
@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2025

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-amdgpu

Author: Abhay Kanhere (AbhayKanhere)

Changes

After PR:#151421 merged following fails in SIFoldOperands showed up.

LLVM :: CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.mfma.gfx90a.ll
LLVM :: CodeGen/AMDGPU/llvm.amdgcn.mfma.gfx90a.ll
LLVM :: CodeGen/AMDGPU/llvm.amdgcn.mfma.ll
LLVM :: CodeGen/AMDGPU/mfma-loop.ll
LLVM :: CodeGen/AMDGPU/rewrite-vgpr-mfma-to-agpr.ll

In Folding code, if folded operand is register ensure earlyClobber is set.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SIFoldOperands.cpp (+4)
diff --git a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
index 6616b30410590..cf9a6f021fd9b 100644
--- a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
@@ -689,6 +689,10 @@ bool SIFoldOperandsImpl::updateOperand(FoldCandidate &Fold) const {
     if (!TII->isOperandLegal(*MI, OpNo, &New))
       return false;
 
+    const MCInstrDesc &MCID = MI->getDesc();
+    if (MCID.getOperandConstraint(OpNo, MCOI::EARLY_CLOBBER) != -1) {
+      MI->getOperand(OpNo).setIsEarlyClobber(true);
+    }
     Old.ChangeToImmediate(*ImmVal);
     return true;
   }

@AbhayKanhere
Copy link
Contributor Author

@arsenm please review.

@shiltian
Copy link
Contributor

shiltian commented Nov 5, 2025

Looks like this can only be reproduced with expensive checkes.

Copy link
Contributor

@shiltian shiltian left a comment

Choose a reason for hiding this comment

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

The fix looks reasonable to me, but I'm not sure if we want to add a test case w/o need of expensive checks.

Co-authored-by: Shilei Tian <i@tianshilei.me>
Fixed tests - added -verify-machineinstrs so fails are visible sooner.
This resolves all fails so far.
removed change in MachineVerifier
@AbhayKanhere AbhayKanhere requested a review from arsenm November 7, 2025 16:40
@AbhayKanhere
Copy link
Contributor Author

AbhayKanhere commented Nov 7, 2025

I would like to merge this ASAP once approved to unblock the fails in nightly runs. thanks!

@AbhayKanhere AbhayKanhere force-pushed the eng/abhay/machine-verify-early-clobber-fix-tests branch from bcdef72 to 2047ef6 Compare November 7, 2025 16:50
@arsenm arsenm merged commit b4b57ad into llvm:main Nov 8, 2025
10 checks passed
vinay-deshmukh pushed a commit to vinay-deshmukh/llvm-project that referenced this pull request Nov 8, 2025
After PR:llvm#151421 merged
following fails in SIFoldOperands showed up.

LLVM :: CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.mfma.gfx90a.ll
LLVM :: CodeGen/AMDGPU/llvm.amdgcn.mfma.gfx90a.ll
LLVM :: CodeGen/AMDGPU/llvm.amdgcn.mfma.ll
LLVM :: CodeGen/AMDGPU/mfma-loop.ll
LLVM :: CodeGen/AMDGPU/rewrite-vgpr-mfma-to-agpr.ll

In Folding code, if folded operand is register ensure earlyClobber is
set.

---------

Co-authored-by: Matt Arsenault <arsenm2@gmail.com>
Co-authored-by: Shilei Tian <i@tianshilei.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants