[PATCH] D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Do we really consider the existing atomic intrinsics to not impose added alignment restrictions?  I'm somewhat concerned that we're going to produce IR here that's either really suboptimal or, worse, unimplementable, just because we over-interpreted some cue about alignment.  I guess it would only be a significant problem on a target where types are frequently under-aligned for what atomics need, which is not typical, or when the user is doing atomics on a field of something like a packed struct.



================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:354
   llvm::Type *Int128PtrTy = Int128Ty->getPointerTo();
-  Destination = CGF.Builder.CreateBitCast(Destination, Int128PtrTy);
-  Address ComparandResult(CGF.Builder.CreateBitCast(ComparandPtr, Int128PtrTy),
-                          CGF.getContext().toCharUnitsFromBits(128));
+  DestAddr = CGF.Builder.CreateBitCast(DestAddr, Int128PtrTy);
+  ComparandAddr = CGF.Builder.CreateBitCast(ComparandAddr, Int128PtrTy);
----------------
Since you're changing this code anyway, please make this do `CreateElementBitCast(DestAddr, Int128Ty)` so that it's address-space-correct.

There are a lot of other lines in the patch that would benefit from the same thing.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3765
     Ptr = Builder.CreateBitCast(Ptr, Int8Ty->getPointerTo(AddrSpace));
+    Address PtrAddr(Ptr, CharUnits::One());
     Value *NewVal = Builder.getInt8(1);
----------------
This should be using `EmitPointerWithAlignment` instead of assuming an alignment of 1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97224