[PATCH] D121229: [AMDGPU] Split FeatureAtomicFaddInsts feature. NFC.

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

> I do not think this is as simple as this.

Is there something wrong with the patch? Really all it is doing is:

1. Setting two predicates on an instruction, like HasAtomicFaddInsts and isGFX940Plus, instead of using compound predicates like HasAtomicFaddInstsGFX940. The hope here is to avoid an N^2 explosion of named predicates.
2. Using HasAtomicFaddNoRtnInsts instead of isGFX90APlus in a couple of places where it makes things more symmetrical.

I could even split this into two separate patches.



================
Comment at: llvm/lib/Target/AMDGPU/FLATInstructions.td:812
 
-let OtherPredicates = [isGFX90APlus] in {
+let OtherPredicates = [HasAtomicFaddRtnInsts] in {
   defm GLOBAL_ATOMIC_ADD_F32 : FLAT_Global_Atomic_Pseudo_RTN <
----------------
Here I switched to using HasAtomicFaddRtnInsts for symmetry with the HasAtomicFaddNoRtnInsts block just above.


================
Comment at: llvm/lib/Target/AMDGPU/FLATInstructions.td:1633
 
-let SubtargetPredicate = HasAtomicFaddInstsGFX940 in {
+let SubtargetPredicate = isGFX940Plus in {
+  // These instructions are encoded differently on gfx90* and gfx940.
----------------
Here I'm only setting the SubtargetPredicate, because I can relying on the Real instruction inheriting the Pseudo instruction's OtherPredicates to pick up the HasAtomicFaddInsts part of the condition.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121229/new/

https://reviews.llvm.org/D121229