jyknight added a comment.

In D97224#2596410 <https://reviews.llvm.org/D97224#2596410>, @rjmccall wrote:

> 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.

For the `__atomic_*` intrinsics, we don't consider those as imposing additional 
alignment restrictions -- currently, we avoid generating the LLVM IR 
instruction if it's below natural alignment, since we couldn't specify 
alignment on the IR instruction. (Now that we have alignment on the LLVM IR 
operations, I'd like to eventually get rid of that logic from Clang, since it's 
also handled by LLVM.)

For other intrinsics (e.g. MSVCIntrin::_InterlockedAnd, 
Builtin::BI__sync_fetch_and_add_4, NVPTX::BI__nvvm_atom_add_gen_i, and the 
others in those 3 function families), we currently entirely ignore the 
alignment, and simply assume the argument is naturally-aligned. So, yes, this 
change could potentially affect the behavior for underaligned types.

So, I could change these to explicitly increase the assumed alignment of their 
arguments, like I did for InterlockedCompareExchange128. My inclination is not 
to do so, however...it doesn't seem like a good idea in general to ignore type 
alignment. But, I'd not be opposed to doing that, if there's a good reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97224

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to