Skip to content

[AMDGPU][True16] remove AsmVOP3OpSel #143465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 11, 2025

Conversation

broxigarchen
Copy link
Contributor

@broxigarchen broxigarchen commented Jun 10, 2025

This is NFC. Clean up the AsmVOP3OpSel field, and use Vop3Base instead.

@broxigarchen broxigarchen changed the title replace opsel profile to vop3base [AMDGPU] replace opsel to vop3base Jun 10, 2025
@broxigarchen broxigarchen changed the title [AMDGPU] replace opsel to vop3base [AMDGPU] remove AsmVOP3OpSel Jun 10, 2025
@broxigarchen broxigarchen changed the title [AMDGPU] remove AsmVOP3OpSel [AMDGPU][True16] remove AsmVOP3OpSel Jun 10, 2025
@broxigarchen broxigarchen reopened this Jun 10, 2025
@broxigarchen broxigarchen marked this pull request as ready for review June 10, 2025 12:27
@broxigarchen broxigarchen requested review from rampitec, arsenm, kosarev and Sisyph and removed request for arsenm June 10, 2025 12:27
@broxigarchen broxigarchen requested a review from arsenm June 10, 2025 12:27
@llvmbot
Copy link
Member

llvmbot commented Jun 10, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Brox Chen (broxigarchen)

Changes

This is NFC. Unified vop3opsel with vop3base, removed AsmVOP3OpSel.


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

3 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.td (+1-43)
  • (modified) llvm/lib/Target/AMDGPU/VOP3Instructions.td (+2-8)
  • (modified) llvm/lib/Target/AMDGPU/VOPInstructions.td (+2-4)
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.td b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
index 2c20475726a48..e74ccbee975ab 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.td
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
@@ -2242,41 +2242,6 @@ class getAsmVOP3P <bit HasDst, int NumSrcArgs, bit HasNeg,
   string ret = dst#src0#src1#src2#opsel#mods#clamp;
 }
 
-// FIXME-TRUE16 AsmVOP3OpSel will be deprecated after all
-// VOP3 16 bit instructions are replaced to true16 format
-class getAsmVOP3OpSel <int NumSrcArgs,
-                       bit HasClamp,
-                       bit HasOMod,
-                       bit Src0HasMods,
-                       bit Src1HasMods,
-                       bit Src2HasMods,
-                       bit HasByteSel = 0,
-                       bit HasBitOp3 = 0> {
-  string dst = "$vdst";
-
-  string isrc0 = !if(!eq(NumSrcArgs, 1), "$src0", "$src0,");
-  string isrc1 = !if(!eq(NumSrcArgs, 1), "",
-                     !if(!eq(NumSrcArgs, 2), " $src1",
-                                             " $src1,"));
-  string isrc2 = !if(!eq(NumSrcArgs, 3), " $src2", "");
-
-  string fsrc0 = !if(!eq(NumSrcArgs, 1), "$src0_modifiers", "$src0_modifiers,");
-  string fsrc1 = !if(!eq(NumSrcArgs, 1), "",
-                     !if(!eq(NumSrcArgs, 2), " $src1_modifiers",
-                                             " $src1_modifiers,"));
-  string fsrc2 = !if(!eq(NumSrcArgs, 3), " $src2_modifiers", "");
-
-  string src0 = !if(Src0HasMods, fsrc0, isrc0);
-  string src1 = !if(Src1HasMods, fsrc1, isrc1);
-  string src2 = !if(Src2HasMods, fsrc2, isrc2);
-
-  string bytesel = !if(HasByteSel, "$byte_sel", "");
-  string clamp = !if(HasClamp, "$clamp", "");
-  string omod = !if(HasOMod, "$omod", "");
-  string bitop3 = !if(HasBitOp3, "$bitop3", "");
-  string ret = dst#", "#src0#src1#src2#bitop3#"$op_sel"#bytesel#clamp#omod;
-}
-
 class getAsmDPP <bit HasDst, int NumSrcArgs, bit HasModifiers, ValueType DstVT = i32> {
   string dst = !if(HasDst,
                    !if(!eq(DstVT.Size, 1),
@@ -2687,14 +2652,7 @@ class VOPProfile <list<ValueType> _ArgVT, bit _EnableClamp = 0> {
    HasSrc2Mods, DstVT, HasFP8ByteSel, HasBitOp3>.ret;
   field string Asm64 = AsmVOP3Base;
   field string AsmVOP3P = getAsmVOP3P<HasDst, NumSrcArgs, HasNeg, HasClamp, HasOpSel>.ret;
-  field string AsmVOP3OpSel = getAsmVOP3OpSel<NumSrcArgs,
-                                              HasClamp,
-                                              HasOMod,
-                                              HasSrc0FloatMods,
-                                              HasSrc1FloatMods,
-                                              HasSrc2FloatMods,
-                                              HasFP8ByteSel,
-                                              HasBitOp3>.ret;
+  field string AsmVOP3OpSel = AsmVOP3Base;
   field string AsmVOP3DPP = getAsmVOP3DPP<AsmVOP3Base>.ret;
   field string AsmVOP3DPP16 = getAsmVOP3DPP16<AsmVOP3Base>.ret;
   field string AsmVOP3DPP8 = getAsmVOP3DPP8<AsmVOP3Base>.ret;
diff --git a/llvm/lib/Target/AMDGPU/VOP3Instructions.td b/llvm/lib/Target/AMDGPU/VOP3Instructions.td
index ef88a379f3680..908325f7ee7c3 100644
--- a/llvm/lib/Target/AMDGPU/VOP3Instructions.td
+++ b/llvm/lib/Target/AMDGPU/VOP3Instructions.td
@@ -626,10 +626,6 @@ def VOP3_CVT_SR_F8_F32_Profile : VOP3_Profile<VOPProfile<[i32, f32, i32, f32]>,
   let HasOpSel = 1;
   let HasFP8DstByteSel = 1;
   let HasFP8ByteSel = 0; // It works as a dst-bytesel, but does not have byte_sel operand.
-  let AsmVOP3OpSel = !subst(", $src2_modifiers", "",
-                            getAsmVOP3OpSel<3, HasClamp, HasOMod,
-                                            HasSrc0FloatMods, HasSrc1FloatMods,
-                                            HasSrc2FloatMods>.ret);
   let AsmVOP3Base = !subst(", $src2_modifiers", "",
                     getAsmVOP3Base<NumSrcArgs, HasDst, HasClamp,
                     HasOpSel, HasOMod, IsVOP3P, HasModifiers, HasModifiers, 0/*Src1Mods*/,
@@ -1066,12 +1062,10 @@ class VOP3_CVT_SCALE_FP4_F16BF16_TiedInput_Profile<VOPProfile P> : VOP3_Profile<
   let HasSrc2 = 0;
   let HasSrc2Mods = 1;
   let HasOpSel = 1;
-  let AsmVOP3OpSel = !subst(", $src2_modifiers", "",
-                            getAsmVOP3OpSel<3, HasClamp, HasOMod,
-                                            HasSrc0FloatMods, HasSrc1FloatMods,
-                                            HasSrc2FloatMods>.ret);
+  let Asm64 = !subst(", $src2_modifiers", "", AsmVOP3Base);
   let HasExtVOP3DPP = 0;
   let HasFP8DstByteSel = 1;
+  let HasFP8ByteSel = 0;
 }
 
 class VOP3_CVT_SCALE_SR_PK_F4_F16BF16_TiedInput_Profile<ValueType Src0Ty> :
diff --git a/llvm/lib/Target/AMDGPU/VOPInstructions.td b/llvm/lib/Target/AMDGPU/VOPInstructions.td
index 4cd845aaa5497..6045f59d1f040 100644
--- a/llvm/lib/Target/AMDGPU/VOPInstructions.td
+++ b/llvm/lib/Target/AMDGPU/VOPInstructions.td
@@ -112,9 +112,7 @@ class VOP3_Pseudo <string opName, VOPProfile P, list<dag> pattern = [],
   bit HasFP8DstByteSel = P.HasFP8DstByteSel;
   bit HasFP4DstByteSel = P.HasFP4DstByteSel;
 
-  let AsmOperands = !if(!and(!not(P.IsTrue16), isVop3OpSel),
-                        P.AsmVOP3OpSel,
-                        !if(!and(isVOP3P, P.IsPacked), P.AsmVOP3P, P.Asm64));
+  let AsmOperands = !if(!and(isVOP3P, P.IsPacked), P.AsmVOP3P, P.Asm64);
 
   let Size = 8;
   let mayLoad = 0;
@@ -1484,7 +1482,7 @@ class VOP3_Profile_Base<VOPProfile P, VOP3Features Features = VOP3_REGULAR> : VO
 
   let HasModifiers =
       !if (Features.IsMAI, 0,
-           !or(Features.IsPacked, Features.HasOpSel, P.HasModifiers));
+           !or(Features.IsPacked, P.HasModifiers));
 }
 
 class VOP3_Profile<VOPProfile P, VOP3Features Features = VOP3_REGULAR> : VOP3_Profile_Base<P, Features> {

@broxigarchen broxigarchen force-pushed the main-merge-true16-cleanup branch from 20aba40 to 67dd5c8 Compare June 10, 2025 14:22
@broxigarchen
Copy link
Contributor Author

rebased

Copy link
Collaborator

@rampitec rampitec left a comment

Choose a reason for hiding this comment

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

LGTM

@broxigarchen broxigarchen merged commit 28a4ed9 into llvm:main Jun 11, 2025
7 checks passed
rorth pushed a commit to rorth/llvm-project that referenced this pull request Jun 11, 2025
This is NFC. Clean up the AsmVOP3OpSel field, and use Vop3Base instead.
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
This is NFC. Clean up the AsmVOP3OpSel field, and use Vop3Base instead.
rampitec added a commit that referenced this pull request Jun 17, 2025
#143465 has removed
getAsmVOP3OpSel and uses getAsmVOP3Base instead, but original
call to getAsmVOP3OpSel was using HasSrc*FloatMods and the
call to getAsmVOP3Base uses HasSrc*Mods. This does not play
well with opsel. An opsel instruction has modifiers in dag but
shall not have them in the asm string.
rampitec added a commit that referenced this pull request Jun 17, 2025
#143465 has removed
getAsmVOP3OpSel and uses getAsmVOP3Base instead, but original
call to getAsmVOP3OpSel was using HasSrc*FloatMods and the
call to getAsmVOP3Base uses HasSrc*Mods. This does not play
well with opsel. An opsel instruction has modifiers in dag but
shall not have them in the asm string.
rampitec added a commit that referenced this pull request Jun 17, 2025
#143465 has removed
getAsmVOP3OpSel and uses getAsmVOP3Base instead, but original
call to getAsmVOP3OpSel was using HasSrc*FloatMods and the
call to getAsmVOP3Base uses HasSrc*Mods. This does not play
well with opsel. An opsel instruction has modifiers in dag but
shall not have them in the asm string.
rampitec added a commit that referenced this pull request Jun 17, 2025
#143465 has removed
getAsmVOP3OpSel and uses getAsmVOP3Base instead, but original
call to getAsmVOP3OpSel was using HasSrc*FloatMods and the
call to getAsmVOP3Base uses HasSrc*Mods. This does not play
well with opsel. An opsel instruction has modifiers in dag but
shall not have them in the asm string.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 17, 2025
llvm/llvm-project#143465 has removed
getAsmVOP3OpSel and uses getAsmVOP3Base instead, but original
call to getAsmVOP3OpSel was using HasSrc*FloatMods and the
call to getAsmVOP3Base uses HasSrc*Mods. This does not play
well with opsel. An opsel instruction has modifiers in dag but
shall not have them in the asm string.
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