[CodeGen] MachineVerifier to check early-clobber constraint #151421
Conversation
Currently MachineVerifier is missing verifying early-clobber operand constraint. The only other machine operand constraint - TiedTo is already verified.
|
Test failures on linux and windows are the same test |
|
@ mbrkusanin I see you have a PR on AMDGPU global Isel. I tried locating the bug (failure with early-clobber not set when G_AMDGPU_MAD_U64_U32 is emitted) and I tried fixing in legalizer llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp line 4091. but that is not sufficient. I could not locate (with quick searches) where this is emitted in Global ISel. any help appreciated. |
Fix the test failure with this PR
AbhayKanhere
force-pushed
the
eng/abhay/machine-verify-early-clobber
branch
from
984f262 to
a0a0504
Compare
|
@llvm/pr-subscribers-backend-amdgpu Author: Abhay Kanhere (AbhayKanhere) ChangesCurrently MachineVerifier is missing verifying early-clobber operand constraint. Full diff: https://github.com/llvm/llvm-project/pull/151421.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 01703fe09b79a..ebef1c9034f4a 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -2325,6 +2325,13 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
report("Missing mayStore flag", MI);
}
+ // Verify earlyClobber def operand
+ if (MCID.getOperandConstraint(0, MCOI::EARLY_CLOBBER) != -1) {
+ if (!MI->getOperand(0).isReg())
+ report("Early clobber must be a register", MI);
+ if (!MI->getOperand(0).isEarlyClobber())
+ report("Missing earlyClobber flag", MI);
+ }
// Debug values must not have a slot index.
// Other instructions must have one, unless they are inside a bundle.
if (LiveInts) {
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp b/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
index 1a63c48e3666c..8a6ef89a4a062 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
@@ -584,6 +584,7 @@ bool AMDGPUInstructionSelector::selectG_AMDGPU_MAD_64_32(
I.setDesc(TII.get(Opc));
I.addOperand(*MF, MachineOperand::CreateImm(0));
I.addImplicitDefUseOperands(*MF);
+ I.getOperand(0).setIsEarlyClobber(true);
return constrainSelectedInstRegOperands(I, TII, TRI, RBI);
}
|
|
This exposed more test fails someone with AMDGPU knowledge should look into whether this is expected. ! 2025-08-04T18:08:20.5815070Z Failed Tests (10): |
|
FYI I fixed these 10 tests locally and found 11 more. .. Wanted some feedback if this is worth fixing or not. |
Currently MachineVerifier is missing verifying early-clobber operand constraint.
The only other machine operand constraint - TiedTo is already verified.