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

Wed Mar 9 06:22:07 PST 2022

foad added inline comments.

================
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.
----------------
foad wrote:
> 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.
I can replace this with isGFX908orGFX90A. I think it makes the patch cleaner, **but** it causes test/CodeGen/AMDGPU/global-atomics-fp-wrong-subtarget.ll to fail because it does this:
```
; Make sure we can encode and don't fail on functions which have
; instructions not actually supported by the subtarget.
```
I have no idea why it does that, or whether this is still required to work for some reason, but at least the current version of this patch preserves this behaviour.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121229