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

Wed Mar 9 05:15:04 PST 2022

foad added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/BUFInstructions.td:2547

-let SubtargetPredicate = HasAtomicFaddInsts in {
-
----------------
rampitec wrote:
> Why is this predicate dropped?
Real instructions copy OtherPredicates from their pseudo, and the pseudos already have the right OtherPredicates = [HasAtomicFaddXXXInsts]. 

================
Comment at: llvm/lib/Target/AMDGPU/FLATInstructions.td:1602

-let SubtargetPredicate = HasAtomicFaddInstsGFX90X in {
+let SubtargetPredicate = isGFX8GFX9NotGFX940 in {
+// These instructions are encoded differently on gfx90* and gfx940.
----------------
rampitec wrote:
> GFX8?
Yeah, I agree this looks strange. I can clean it up if you like. I think we get away with it because these instructions also pick up OtherPredicates = [HasAtomicFaddXXXInsts] from their pseudos, and those features are never set on GFX8.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121229