[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:665
+  if (Ty->isIntegerTy()) {
+    BitWidth = dyn_cast<IntegerType>(Ty)->getBitWidth();
+  }
----------------
pravinjagtap wrote:
> Is it safe to get BitWidth like this ? We dont need this for `float` types
Simpler to call Ty->getPrimitiveSizeInBits() unconditionally.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp:760
   }
-  Mbcnt = B.CreateIntCast(Mbcnt, Ty, false);
+  Mbcnt = !isAtomicFloatingPointTy ? B.CreateIntCast(Mbcnt, Ty, false) : Mbcnt;
 
----------------
Might be clearer as:
`Mbcnt = isAtomicFloatingPointTy ? B.CreateUIToFP(Mbcnt, Ty) : B.CreateIntCast(Mbcnt, Ty, false);`
(instead of doing the fp cast on line 996) since in both cases we want to convert Mbcnt to type Ty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156301