Skip to content
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

AMDGPU: Copy SubtargetPredicate from pseudo to real for dpp16 and dpp8 #84517

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

changpeng
Copy link
Contributor

We usually expect to copy SubtargetPredicate (and OtherPredicates) from pseudo to real. However, in dpp16 and dpp8, there are assignments like SubtargetPredicate = HasDPP/HasDPP16/HasDpp8. These assignments override predicates copied from pseudo, and thus the predicates used to define pseudo get lost.

Losing predicates is a subtle issue usually not easy to be found. It may result in instructions being generated on GPUs that do not support the features to generate them. #84354 addressed one of such issues, and inspired this work.

Fortunately, we found that the assignment of SubtargetPredicate usually comes together with assignment of AssemblerPredicate, and with the same value. For example:
let AssemblerPredicate = HasDPP16;
let SubtargetPredicate = HasDPP16;
One of them is redundant and can be removed.

In this work, we remove the redundant assignment of SubtargetPredicate, and then copy it from pseudo for VOP*_DPP and VOP*_DPP8. With this change, we can safely use SubtargetPredicate to define pseudo instructions.

  We usually expect to copy SubtargetPredicate (and OtherPredicates) from
pseudo to real. However, in dpp16 and dpp8, there are assignments like
SubtargetPredicate = HasDPP/HasDPP16/HasDpp8. These assignments override
predicates copied from pseudo, and thus the predicates used to define pseudo
get lost.

 Losing predicates is a subtle issue usually not easy to be found. It may
result in instructions being generated on GPUs that do not support the
features to generate them. #84354
addressed one of such issues, and inspired this work.

  Fortunately, we found that the assignment of SubtargetPredicate usually
comes together with assignment of AssemblerPredicate, and with the same
value. For example:
  let AssemblerPredicate = HasDPP16;
  let SubtargetPredicate = HasDPP16;
One of them is redundant and can be removed.

  In this work, we remove the redundant assignment of SubtargetPredicate, and
then copy it from pseudo for VOP*_DPP and VOP*_DPP8. With this change, we
can safely use SubtargetPredicate to define pseudo instructions.
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 8, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Changpeng Fang (changpeng)

Changes

We usually expect to copy SubtargetPredicate (and OtherPredicates) from pseudo to real. However, in dpp16 and dpp8, there are assignments like SubtargetPredicate = HasDPP/HasDPP16/HasDpp8. These assignments override predicates copied from pseudo, and thus the predicates used to define pseudo get lost.

Losing predicates is a subtle issue usually not easy to be found. It may result in instructions being generated on GPUs that do not support the features to generate them. #84354 addressed one of such issues, and inspired this work.

Fortunately, we found that the assignment of SubtargetPredicate usually comes together with assignment of AssemblerPredicate, and with the same value. For example:
let AssemblerPredicate = HasDPP16;
let SubtargetPredicate = HasDPP16;
One of them is redundant and can be removed.

In this work, we remove the redundant assignment of SubtargetPredicate, and then copy it from pseudo for VOP*_DPP and VOP*_DPP8. With this change, we can safely use SubtargetPredicate to define pseudo instructions.


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

4 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/VOP1Instructions.td (+2-1)
  • (modified) llvm/lib/Target/AMDGPU/VOP2Instructions.td (+2-1)
  • (modified) llvm/lib/Target/AMDGPU/VOP3PInstructions.td (+2-1)
  • (modified) llvm/lib/Target/AMDGPU/VOPInstructions.td (+2-4)
diff --git a/llvm/lib/Target/AMDGPU/VOP1Instructions.td b/llvm/lib/Target/AMDGPU/VOP1Instructions.td
index dbb1977183d190..2341e0d9d32bb4 100644
--- a/llvm/lib/Target/AMDGPU/VOP1Instructions.td
+++ b/llvm/lib/Target/AMDGPU/VOP1Instructions.td
@@ -730,6 +730,7 @@ class VOP1_DPP<bits<8> op, VOP1_DPP_Pseudo ps, VOPProfile p = ps.Pfl, bit isDPP1
   let SchedRW = ps.SchedRW;
   let Uses = ps.Uses;
   let TRANS = ps.TRANS;
+  let SubtargetPredicate = ps.SubtargetPredicate;
   let OtherPredicates = ps.OtherPredicates;
 
   bits<8> vdst;
@@ -743,7 +744,6 @@ class VOP1_DPP16<bits<8> op, VOP1_DPP_Pseudo ps, int subtarget, VOPProfile p = p
     VOP1_DPP<op, ps, p, 1>,
     SIMCInstr <ps.PseudoInstr, subtarget> {
   let AssemblerPredicate = HasDPP16;
-  let SubtargetPredicate = HasDPP16;
 }
 
 class VOP1_DPP16_Gen<bits<8> op, VOP1_DPP_Pseudo ps, GFXGen Gen, VOPProfile p = ps.Pfl> :
@@ -758,6 +758,7 @@ class VOP1_DPP8<bits<8> op, VOP1_Pseudo ps, VOPProfile p = ps.Pfl> :
   let Defs = ps.Defs;
   let SchedRW = ps.SchedRW;
   let Uses = ps.Uses;
+  let SubtargetPredicate = ps.SubtargetPredicate;
   let OtherPredicates = ps.OtherPredicates;
 
   bits<8> vdst;
diff --git a/llvm/lib/Target/AMDGPU/VOP2Instructions.td b/llvm/lib/Target/AMDGPU/VOP2Instructions.td
index 53578682e00246..8a92aa8228f121 100644
--- a/llvm/lib/Target/AMDGPU/VOP2Instructions.td
+++ b/llvm/lib/Target/AMDGPU/VOP2Instructions.td
@@ -1259,7 +1259,7 @@ class Base_VOP2_DPP16<bits<6> op, VOP2_DPP_Pseudo ps,
                  string opName = ps.OpName, VOPProfile p = ps.Pfl> :
     VOP2_DPP<op, ps, opName, p, 1> {
   let AssemblerPredicate = HasDPP16;
-  let SubtargetPredicate = HasDPP16;
+  let SubtargetPredicate = ps.SubtargetPredicate;
   let OtherPredicates = ps.OtherPredicates;
 }
 
@@ -1294,6 +1294,7 @@ class VOP2_DPP8<bits<6> op, VOP2_Pseudo ps,
   let Inst{30-25} = op;
   let Inst{31}    = 0x0;
 
+  let SubtargetPredicate = ps.SubtargetPredicate;
   let OtherPredicates = ps.OtherPredicates;
 }
 
diff --git a/llvm/lib/Target/AMDGPU/VOP3PInstructions.td b/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
index e1131bbb78d3fc..4ca2835eea5898 100644
--- a/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
+++ b/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
@@ -1353,7 +1353,7 @@ class VOP3P_DPP16<bits<7> op, VOP_DPP_Pseudo ps, int subtarget,
   let SchedRW = ps.SchedRW;
   let Uses = ps.Uses;
   let AssemblerPredicate = HasDPP16;
-  let SubtargetPredicate = HasDPP16;
+  let SubtargetPredicate = ps.SubtargetPredicate;
   let OtherPredicates = ps.OtherPredicates;
   let IsPacked = ps.IsPacked;
 }
@@ -1364,6 +1364,7 @@ class VOP3P_DPP8_Base<bits<7> op, VOP_Pseudo ps, string opName = ps.OpName>
   let Defs = ps.Defs;
   let SchedRW = ps.SchedRW;
   let Uses = ps.Uses;
+  let SubtargetPredicate = ps.SubtargetPredicate;
   let OtherPredicates = ps.OtherPredicates;
   let IsPacked = ps.IsPacked;
 }
diff --git a/llvm/lib/Target/AMDGPU/VOPInstructions.td b/llvm/lib/Target/AMDGPU/VOPInstructions.td
index fa8d46608f5d11..a6272e946c5168 100644
--- a/llvm/lib/Target/AMDGPU/VOPInstructions.td
+++ b/llvm/lib/Target/AMDGPU/VOPInstructions.td
@@ -832,7 +832,6 @@ class VOP_DPP_Pseudo <string OpName, VOPProfile P, list<dag> pattern=[],
   string AsmOperands = asmOps;
 
   let AsmMatchConverter = !if(P.HasModifiers, "cvtDPP", "");
-  let SubtargetPredicate = !if(P.HasExt64BitDPP, HasDPALU_DPP, HasDPP);
   let AssemblerPredicate = !if(P.HasExt64BitDPP, HasDPALU_DPP, HasDPP);
   let AsmVariantName = !if(P.HasExtDPP, AMDGPUAsmVariants.DPP,
                                         AMDGPUAsmVariants.Disable);
@@ -903,7 +902,6 @@ class VOP_DPP_Base <string OpName, VOPProfile P,
   let Size = 8;
 
   let AsmMatchConverter = !if(P.HasModifiers, "cvtDPP", "");
-  let SubtargetPredicate = !if(P.HasExt64BitDPP, HasDPALU_DPP, HasDPP);
   let AssemblerPredicate = !if(P.HasExt64BitDPP, HasDPALU_DPP, HasDPP);
   let AsmVariantName = !if(P.HasExtDPP, AMDGPUAsmVariants.DPP,
                                         AMDGPUAsmVariants.Disable);
@@ -993,7 +991,6 @@ class VOP_DPP8_Base<string OpName, VOPProfile P, dag InsDPP8 = P.InsDPP8, string
   let Size = 8;
 
   let AsmMatchConverter = "cvtDPP8";
-  let SubtargetPredicate = HasDPP8;
   let AssemblerPredicate = HasDPP8;
   let AsmVariantName = AMDGPUAsmVariants.DPP;
   let Constraints = !if(P.NumSrcArgs, P.TieRegDPP # " = $vdst", "");
@@ -1340,7 +1337,7 @@ class Base_VOP3_DPP16<bits<10> op, VOP_DPP_Pseudo ps, string opName = ps.OpName>
   let SchedRW = ps.SchedRW;
   let Uses = ps.Uses;
   let AssemblerPredicate = HasDPP16;
-  let SubtargetPredicate = HasDPP16;
+  let SubtargetPredicate = ps.SubtargetPredicate;
   let OtherPredicates = ps.OtherPredicates;
 }
 
@@ -1366,6 +1363,7 @@ class Base_VOP3_DPP8<bits<10> op, VOP_Pseudo ps, string opName = ps.OpName>
   let SchedRW = ps.SchedRW;
   let Uses = ps.Uses;
 
+  let SubtargetPredicate = ps.SubtargetPredicate;
   let OtherPredicates = ps.OtherPredicates;
 }
 

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, thanks!

@changpeng changpeng merged commit 839a8fe into llvm:main Mar 8, 2024
5 of 6 checks passed
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.

None yet

3 participants