[PATCH] D156301: [AMDGPU] Support FAdd/FSub global atomics in AMDGPUAtomicOptimizer.

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp:865
   // have 0 active lanes below us, so that will be the only one to progress.
-  Value *const Cond = B.CreateICmpEQ(Mbcnt, B.getIntN(TyBitWidth, 0));
+  Value *const Cond = B.CreateICmpEQ(Mbcnt, B.getInt32(0));
 
----------------
pravinjagtap wrote:
> foad wrote:
> > pravinjagtap wrote:
> > > I hope, this stops 64 bit comparisons for 64 bit atomic values. Please check effect of this in `llvm/test/CodeGen/AMDGPU/atomic_optimizations_local_pointer.ll`
> > I don't actually see any 64-bit cmp instructions in that test, even before your patch. I guess we already managed to shrink them back to 32-bit comparisons.
> Having 32-bit comparison here for all the cases (int, long, float, wavefront size 32/64) is fine right ? Or do I need to revert this change?
It is fine. We are talking about the `laneid == 0` comparison, which should always be 32-bit even for a 64-bit atomic, since the laneid is just a small integer in the range 0..63.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156301