[PATCH] D37221: [AtomicExpand][X86] Let atomic expand generate inline sequence for unaligned load/store of atomic primitive integer types on x86_64

[Wei Mi via llvm-commits]

You are right, so this is an incorrect fix.

The correct fix is to increase the alignment of atomic variable. For
the case in PR34347, GCC aligns the atomic variable to 8 bytes while
Clang aligns the atomic variable to 4 bytes, and that leads to better
code in GCC. I will try that.


On Mon, Aug 28, 2017 at 11:05 AM, James Y Knight <jyknight at google.com> wrote:
> I'm pretty sure the x86-64 manual said this was *not* guaranteed for
> accesses that cross a cacheline. (On vacation, so am not going to lookup
> quote)
>
> Do you read something that makes you think this is actually safe?
>
>
> On Aug 28, 2017 1:36 PM, "Wei Mi via Phabricator" <reviews at reviews.llvm.org>
> wrote:
>
> wmi created this revision.
> Herald added a subscriber: sanjoy.
>
> The patch is to fix PR34347.
>
> Now llvm generates library calls of atomic_load/store for unaligned
> load/store of atomic primitive integer type vars on x86_64. Since x86_64
> support unaligned atomic load/store for primitive integers, this is not
> efficient. The patch fixes that.
>
> To fix PR34347, change in clang is also needed. That will be included in
> another patch.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D37221
>
> Files:
>   include/llvm/Target/TargetLowering.h
>   lib/CodeGen/AtomicExpandPass.cpp
>   lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>   lib/Target/X86/X86ISelLowering.cpp
>   lib/Target/X86/X86ISelLowering.h
>   test/Transforms/AtomicExpand/X86/expand-unaligned-expansion.ll
>
>