[PATCH] D12543: [x86] fix allowsMisalignedMemoryAccesses() for 8-byte and smaller accesses

[Sanjay Patel via llvm-commits]

spatel added a comment.

Thanks, Quentin!

In http://reviews.llvm.org/D12543#237849, @qcolombet wrote:

> > The overlapping accesses may be a separate bug.
>
>
> I do not know if this is a separate bug, but at least it does not seem related to your changes, i.e., it happened before, right :).


That's correct - the overlapping codegen isn't created by this patch, although it may occur more often after this change.
It comes from SelectionDAG's getMem{cpy/set/move} which always pass AllowOverlap = true to getMem*LoadsAndStores().

> What is the PR for that?

>  Do you plan to follow-up on that?


I'm not aware of a PR for this. It just looks wrong to me. :)
But I haven't run any tests to prove there's a perf problem. I was hoping that someone looking at this review could tell me if there really is nothing wrong with overlapping accesses like that. Either way, I'll try some experiments.

The constant store case in memcpy-2.ll looks particularly sketchy:

  movabsq  $33909456017848440, %rax ## imm = 0x78787878787878
  movq     %rax, -10(%rsp)
  movabsq  $8680820740569200760, %rax ## imm = 0x7878787878787878
  movq     %rax, -16(%rsp)

We've created an extra 64-bit constant, so that's wasteful at the least. And does this code now have a race condition because we're storing zeros to memory where there should not be from the time the first store completes until the second overrwrites those zeros?


http://reviews.llvm.org/D12543