[PATCH] D12338: Make `llvm::expandAtomicRMWToCmpXchg`'s initial load atomic.

[Richard Diamond via llvm-commits]

DiamondLovesYou marked an inline comment as done.
DiamondLovesYou added a comment.

In http://reviews.llvm.org/D12338#245624, @jyasskin wrote:

> In http://reviews.llvm.org/D12338#245398, @jfb wrote:
>
> > @jyasskin I took a look at implementing what we discussed <http://reviews.llvm.org/D12338#239615>, and it's quite a bit uglier than I though it would be:
> >
> > - We want the same value that factors into `ATOMIC_*_LOCK_FREE`, which is `MaxAtomicInlineWidth` from `tools/clang/lib/Basic/Targets.cpp` (each `Target` should define a value there, or gets `0` by default).
>
>
> Richard and I don't think you want that value. On x86-64, the maximum width here is actually 128 bits, but you want to split loads down to 64 bits.
>
> > - We can pass this to the backend through LLVM's `TargetOptions`, which clang's `BackendUtil.cpp` can initialize.
>
> > - `AtomicExpandPass` has a `TargetMachine`, which has `TargetOptions`.
>
> > 
>
> >   This will work for code that come straight from C++, but won't work for code that only uses opt: clang has the information we need, and LLVM doesn't. This is pretty much something that should be in the `DataLayout` instead, which is a *very* involved change! WDYT? Am I missing something obvious?
>
>
> You're probably right, but I don't know enough about the backends to be sure. It does seem like the backend should know the widths of atomics it supports.


It seems to me that this is a x86-64 only issue, in that only x86-64 wants to split >64bit atomic loads in two. Perhaps we could add a load attribute, ie `tearable`, that would signal to the backend that this particular load could be torn without violating frontend assumptions? The ability to tear this initial load seems to me to be a property of this particular transform, and not of general IR (obviously). WDYT?


http://reviews.llvm.org/D12338