[PATCH] D129690: [LLVM][AMDGPU] Specialize 32-bit atomic fadd instruction for generic address space

Wed Oct 5 08:18:06 PDT 2022

tianshilei1992 added a comment.

In D129690#3811220 <https://reviews.llvm.org/D129690#3811220>, @Petar.Avramovic wrote:

> I am not sure about changes in SIISelLowering.cpp, it looks correct for gfx90a but not for gfx908. Can you rebase on top of D131560 <https://reviews.llvm.org/D131560>?
> There are some additions to when rmw fadd atomics are expanded.
> If I am reading this correctly, flat f32 fadd that is non-system scope and function has "amdgpu-unsafe-fp-atomics"="true" will use expand from this patch on gfx908 no-rtn fadd and gfx90a?
> Remaining two targets (gfx940 and gfx11) that have global fadd f32 also have flat fadd f32 instructions.
> Can you also update summary, there are a few targets that have flat/global fadd.
> You are changing way of expansion on targets that have global fadd but does not have flat fadd instruction (if atomic is non-system scope and function has "amdgpu-unsafe-fp-atomics"="true" attribute)?
> Also there is no check if target has hasLDSFPAtomicAdd before using  AtomicExpansionKind::Expand (targets affected by this change have it but should probably add feature check before expanding)

Thanks for the info. I rebased the patch and refined the logic to determine. Does it look right now?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129690