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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

I'd still like to have an IR to IR test in test/Transforms/AtomicExpand



================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:13040
+  Value *NewVal = Builder.CreateFAdd(LoadedPrivate, Val, "val.new");
+  Builder.CreateStore(NewVal, Addr);
+  Builder.CreateBr(PhiBB);
----------------
tianshilei1992 wrote:
> arsenm wrote:
> > Pass through AA mteadata?
> Can you expatiate it? I didn't get it.
It's probably not important, but you can forward any aliasing metadata through from the original atomic to the new memory operation. 


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:13002
+  //    %cast.private = addrspacecast float* %addr, float addrspace(5)*
+  //    %loaded.private = load float* %cast.private
+  //    %new.val = fadd %loaded_private, %val
----------------
put addrspace(5) here


================
Comment at: llvm/test/CodeGen/AMDGPU/atomicrmw-expand.ll:45-46
+
+ attributes #0 = { "amdgpu-unsafe-fp-atomics"="true" "target-features"="+flat-address-space,+gfx9-insts" }
+ attributes #1 = { "amdgpu-unsafe-fp-atomics"="true" "target-features"="+flat-address-space,+gfx9-insts" }
----------------
These two attribute groups are the same. Also you can drop the target-features


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129690