[AMDGPU] Simplify getIns64. NFCI. #139981
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6c7c212 to
5ba584e
Compare
|
@llvm/pr-subscribers-backend-amdgpu Author: Stanislav Mekhanoshin (rampitec) ChangesThis big switch is unmaintainable and buggy. In particular it unconditionally Full diff: https://github.com/llvm/llvm-project/pull/139981.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.td b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
index 79667e5ff9285..203992e3853d0 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.td
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
@@ -1972,81 +1972,23 @@ class getIns64 <RegisterOperand Src0RC, RegisterOperand Src1RC,
RegisterOperand Src2RC, int NumSrcArgs,
bit HasClamp, bit HasModifiers, bit HasSrc2Mods, bit HasOMod,
Operand Src0Mod, Operand Src1Mod, Operand Src2Mod> {
-
- dag ret =
- !if (!eq(NumSrcArgs, 0),
- // VOP1 without input operands (V_NOP, V_CLREXCP)
- (ins),
- /* else */
- !if (!eq(NumSrcArgs, 1),
- !if (HasModifiers,
- // VOP1 with modifiers
- !if(HasOMod,
- (ins Src0Mod:$src0_modifiers, Src0RC:$src0,
- Clamp0:$clamp, omod0:$omod),
- !if (HasClamp,
- (ins Src0Mod:$src0_modifiers, Src0RC:$src0, Clamp0:$clamp),
- (ins Src0Mod:$src0_modifiers, Src0RC:$src0)))
- /* else */,
- // VOP1 without modifiers
- !if(HasOMod,
- (ins Src0RC:$src0, Clamp0:$clamp, omod0:$omod),
- !if (HasClamp,
- (ins Src0RC:$src0, Clamp0:$clamp),
- (ins Src0RC:$src0)))
- /* endif */ ),
- !if (!eq(NumSrcArgs, 2),
- !if (HasModifiers,
- // VOP 2 with modifiers
- !if(HasOMod,
- (ins Src0Mod:$src0_modifiers, Src0RC:$src0,
- Src1Mod:$src1_modifiers, Src1RC:$src1,
- Clamp0:$clamp, omod0:$omod),
- !con((ins Src0Mod:$src0_modifiers, Src0RC:$src0,
- Src1Mod:$src1_modifiers, Src1RC:$src1),
- !if(HasClamp, (ins Clamp0:$clamp), (ins))))
- /* else */,
- // VOP2 without modifiers
- !if (HasClamp,
- (ins Src0RC:$src0, Src1RC:$src1, Clamp0:$clamp),
- (ins Src0RC:$src0, Src1RC:$src1))
-
- /* endif */ )
- /* NumSrcArgs == 3 */,
- !if (HasModifiers,
- !if (HasSrc2Mods,
- // VOP3 with modifiers
- !if (HasOMod,
- (ins Src0Mod:$src0_modifiers, Src0RC:$src0,
- Src1Mod:$src1_modifiers, Src1RC:$src1,
- Src2Mod:$src2_modifiers, Src2RC:$src2,
- Clamp0:$clamp, omod0:$omod),
- !if (HasClamp,
- (ins Src0Mod:$src0_modifiers, Src0RC:$src0,
- Src1Mod:$src1_modifiers, Src1RC:$src1,
- Src2Mod:$src2_modifiers, Src2RC:$src2,
- Clamp0:$clamp),
- (ins Src0Mod:$src0_modifiers, Src0RC:$src0,
- Src1Mod:$src1_modifiers, Src1RC:$src1,
- Src2Mod:$src2_modifiers, Src2RC:$src2))),
- // VOP3 with modifiers except src2
- !if (HasOMod,
- (ins Src0Mod:$src0_modifiers, Src0RC:$src0,
- Src1Mod:$src1_modifiers, Src1RC:$src1,
- Src2RC:$src2, Clamp0:$clamp, omod0:$omod),
- !if (HasClamp,
- (ins Src0Mod:$src0_modifiers, Src0RC:$src0,
- Src1Mod:$src1_modifiers, Src1RC:$src1,
- Src2RC:$src2, Clamp0:$clamp),
- (ins Src0Mod:$src0_modifiers, Src0RC:$src0,
- Src1Mod:$src1_modifiers, Src1RC:$src1,
- Src2RC:$src2))))
- /* else */,
- // VOP3 without modifiers
- !if (HasClamp,
- (ins Src0RC:$src0, Src1RC:$src1, Src2RC:$src2, Clamp0:$clamp),
- (ins Src0RC:$src0, Src1RC:$src1, Src2RC:$src2))
- /* endif */ ))));
+ dag src0 = !if (HasModifiers,
+ (ins Src0Mod:$src0_modifiers, Src0RC:$src0),
+ (ins Src0RC:$src0));
+ dag src1 = !if(!ge(NumSrcArgs, 2),
+ !if (HasModifiers,
+ (ins Src1Mod:$src1_modifiers, Src1RC:$src1),
+ (ins Src1RC:$src1)),
+ (ins));
+ dag src2 = !if(!ge(NumSrcArgs, 3),
+ !if (HasSrc2Mods,
+ (ins Src2Mod:$src2_modifiers, Src2RC:$src2),
+ (ins Src2RC:$src2)),
+ (ins));
+ dag clamp = !if (HasClamp, (ins Clamp0:$clamp), (ins));
+ dag omod = !if(HasOMod, (ins omod0:$omod), (ins));
+
+ dag ret = !con(src0, src1, src2, clamp, omod);
}
class getInsVOP3Base<RegisterOperand Src0RC, RegisterOperand Src1RC,
diff --git a/llvm/lib/Target/AMDGPU/VOP2Instructions.td b/llvm/lib/Target/AMDGPU/VOP2Instructions.td
index 30cef69aa29c4..0c7e20fc1ebf3 100644
--- a/llvm/lib/Target/AMDGPU/VOP2Instructions.td
+++ b/llvm/lib/Target/AMDGPU/VOP2Instructions.td
@@ -439,7 +439,7 @@ class VOP_MAC <ValueType vt0, ValueType vt1=vt0> : VOPProfile <[vt0, vt1, vt1, v
// Src2 must accept the same operand types as vdst, namely VGPRs only
let Src2RC64 = getVOP3VRegForVT<Src2VT, IsTrue16, !not(IsRealTrue16)>.ret;
let Ins64 = getIns64<Src0RC64, Src1RC64, Src2RC64, 3,
- 0, HasModifiers, HasModifiers, HasOMod,
+ HasClamp, HasModifiers, HasModifiers, HasOMod,
Src0Mod, Src1Mod, Src2Mod>.ret;
let InsDPP = (ins Src0ModDPP:$src0_modifiers, Src0DPP:$src0,
Src1ModDPP:$src1_modifiers, Src1DPP:$src1,
@@ -448,7 +448,7 @@ class VOP_MAC <ValueType vt0, ValueType vt1=vt0> : VOPProfile <[vt0, vt1, vt1, v
DppBankMask:$bank_mask, DppBoundCtrl:$bound_ctrl);
let InsDPP16 = !con(InsDPP, (ins Dpp16FI:$fi));
let InsVOP3Base = getInsVOP3Base<Src0VOP3DPP, Src1VOP3DPP, RegisterOperand<VGPR_32>, 3,
- 0, HasModifiers, HasModifiers, HasOMod,
+ HasClamp, HasModifiers, HasModifiers, HasOMod,
Src0ModVOP3DPP, Src1ModVOP3DPP, Src2Mod, HasOpSel>.ret;
// We need a dummy src2 tied to dst to track the use of that register for s_delay_alu
let InsVOPDX = (ins Src0RC32:$src0X, Src1RC32:$vsrc1X, VGPRSrc_32:$src2X);
|
shiltian
approved these changes
May 15, 2025
|
Interesting... Failure only happens in a debug build and it is not an assert... |
arsenm
approved these changes
May 15, 2025
5ba584e to
e64135b
Compare
OK, it did not factor v_nop w/o any operands. Fixed. |
arsenm
approved these changes
May 15, 2025
This big switch is unmaintainable and buggy. In particular it unconditionally adds clamp if there is omod to VOP3.
e64135b to
17bac1c
Compare
I mean... going down the rabbit hole of t16, you will inevitable come to this place. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This big switch is unmaintainable and buggy. In particular it unconditionally
adds clamp if there is omod to VOP3.