[PATCH] D102255: [SelectionDAG] Generate scoped AA metadata when lowering memcpy.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D102255#2751961 <https://reviews.llvm.org/D102255#2751961>, @hliao wrote:

> In D102255#2751896 <https://reviews.llvm.org/D102255#2751896>, @nikic wrote:
>
>> Do I understand correctly that this patch is trying to claim that the memcpy src and dst do not alias through scoped alias metadata? If so, I'm afraid this is incorrect. Our contract for `llvm.memcpy` requires that src/dst are either NoAlias or MustAlias (but not PartialAlias). From LangRef:
>>
>>> The ‘llvm.memcpy.*’ intrinsics copy a block of memory from the source location to the destination location, which must either be equal or non-overlapping.
>
> The langref says "The ‘llvm.memcpy.*’ intrinsics copy a block of memory from the source location to the destination location, which must either be equal or non-overlapping. It copies “len” bytes of memory over. If the argument is known to be aligned to some boundary, this can be specified as an attribute on the argument.". It says that src and dst are either equal or non-overlapping. For the equal case, they should be eliminated earlier.

Generally, you do not know whether they are equal or not.

> Even if there are remaining ones, claiming them 'NoAlias' doesn't have correctness issue when copying them. It won't cause correctness issue if we schedule those loads and stores freely.

I agree that rescheduling the loads/stores is correct even if src and dst are equal. However, the metadata itself is still incorrect: It will claim that the loads/stores are NoAlias, even though they are actually MustAlias.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102255