[PATCH] D80804: [AMDGPU] Expose llvm atomic inc/dec instructions as clang builtins for AMDGPU target

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14524
+        BuiltinAtomicOp,
+        {Ptr->getType()->getPointerElementType(), Ptr->getType()});
 
----------------
Should not rely on pointer element type, these are always i32 I think


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14529-14530
+
+      // llvm.amdgcn.atomic.inc and llvm.amdgcn.atomic.dec expects ordering and
+      // scope as unsigned values
+      Value *MemOrder = Builder.getInt32(static_cast<int>(AO));
----------------
We should fix this (or move these into atomicrmw)


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14534
+
+      Value *IsVolatile = EmitScalarExpr(E->getArg(4));
+
----------------
Should this come from whether the builtin was called with a volatile pointer rather than being an explicit parameter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80804