[AMDGPU] Remove unnecessary predicates from aliases. NFC.

[jayfoad]

So long as the target of the alias is predicated with HasImageInsts or
similar, the alias itself does not need this predicate.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Jay Foad (jayfoad)

Changes

So long as the target of the alias is predicated with HasImageInsts or
similar, the alias itself does not need this predicate.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/EXPInstructions.td (+1-1)
• (modified) llvm/lib/Target/AMDGPU/MIMGInstructions.td (+1-1)

diff --git a/llvm/lib/Target/AMDGPU/EXPInstructions.td b/llvm/lib/Target/AMDGPU/EXPInstructions.td
index b73b83031af0d..9bfbd1405ba6c 100644
--- a/llvm/lib/Target/AMDGPU/EXPInstructions.td
+++ b/llvm/lib/Target/AMDGPU/EXPInstructions.td
@@ -117,7 +117,7 @@ multiclass EXP_Real_gfx11 {
 multiclass VEXPORT_Real_gfx12 {
   defvar ps = !cast<EXP_Pseudo>(NAME);
   def _gfx12 : EXP_Real_Row<ps, SIEncodingFamily.GFX12, "export">,
-    EXPe_Row, MnemonicAlias<"exp", "export">, Requires<[isGFX12Plus, HasExportInsts]> {
+    EXPe_Row, MnemonicAlias<"exp", "export">, Requires<[isGFX12Plus]> {
     let AssemblerPredicate = isGFX12Only;
     let DecoderNamespace = "GFX12";
     let row = ps.row;
diff --git a/llvm/lib/Target/AMDGPU/MIMGInstructions.td b/llvm/lib/Target/AMDGPU/MIMGInstructions.td
index 23e8be0d5e45e..73a64a8c949e1 100644
--- a/llvm/lib/Target/AMDGPU/MIMGInstructions.td
+++ b/llvm/lib/Target/AMDGPU/MIMGInstructions.td
@@ -967,7 +967,7 @@ class VIMAGE_Atomic_gfx12_Renamed<mimgopc op, string opcode, string renamed,
                                   RegisterClass DataRC, int num_addrs,
                                   bit enableDisasm = 0>
    : VIMAGE_Atomic_gfx12<op, renamed, DataRC, num_addrs, enableDisasm>,
-     MnemonicAlias<opcode, renamed>, Requires<[isGFX12Plus, HasImageInsts]>;
+     MnemonicAlias<opcode, renamed>, Requires<[isGFX12Plus]>;
 
 multiclass MIMG_Atomic_Addr_Helper_m <mimgopc op, string asm,
                                       RegisterClass data_rc,

[Sisyph]

I think that since Requires is the last defined class in def _gfx12 ..., only the predicates in Requires will apply to the whole instruction definition. So is it Ok to remove HasExportInsts from this instruction? It might be if isGFX12Plus implies HasExportInsts

[Sisyph]

Is this PR related to #89288 ? I think in practice the export pseudo needs HasExportInsts for selection, the Export Real needs isGFX12Plus for lowering to MCInst, and the MnemonicAlias needs isGFX12Plus for having the alias on the right targets

[jayfoad]

I think that since Requires is the last defined class in def _gfx12 ..., only the predicates in Requires will apply to the whole instruction definition.

Oh, good point. I had not noticed that the instruction and alias used the same def. I am tempted to split the alias into its own anonymous def.

So is it Ok to remove HasExportInsts from this instruction? It might be if isGFX12Plus implies HasExportInsts

I suspect that nothing ever looks at the predicates on a Real instruction. Even the predicates on a Pseudo instruction don't seem to have much effect.

Is this PR related to #89288 ?

Yes.

I think in practice the export pseudo needs HasExportInsts for selection,

Possibly, but I suspect only the predicates on the pattern matter for this, not the predicates on the selected instruction.

the Export Real needs isGFX12Plus for lowering to MCInst,

Maybe. I don't know about that.

and the MnemonicAlias needs isGFX12Plus for having the alias on the right targets

Yes.

[jayfoad]

I think that since Requires is the last defined class in def _gfx12 ..., only the predicates in Requires will apply to the whole instruction definition.

Oh, good point. I had not noticed that the instruction and alias used the same def. I am tempted to split the alias into its own anonymous def.

Or if you are close to merging #89288 then I'll wait for you to merge that one first.

[Sisyph]

Or if you are close to merging #89288 then I'll wait for you to merge that one first.

I just merged it.

[jayfoad]

Or if you are close to merging #89288 then I'll wait for you to merge that one first.

I just merged it.

Thanks. I rebased this patch which fixes the "predicates in Requires will apply to the whole instruction" problem.