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

[Michael Liao via Phabricator via llvm-commits]

hliao added a comment.

In D102255#2766721 <https://reviews.llvm.org/D102255#2766721>, @jeroen.dobbelaere wrote:

> In D102255#2765788 <https://reviews.llvm.org/D102255#2765788>, @jeroen.dobbelaere wrote:
>
>> In D102255#2755372 <https://reviews.llvm.org/D102255#2755372>, @nikic wrote:
>>
>>> 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.
>>
>> The proposed version introduces 2 scopes, assigning all store to one scope and all loads to another.
>>
>> I order to avoid the `MustAlias` vs `NoAlias` problem, we could introduce a scope per load-store pair. Of course, that might be a lot of extra scopes..
>
> This was discussed earlier today in LLVM's AA Technical call. This method should work, but one possible gotcha came up: sometimes a memcpy lowering results in overlapping load/stores. Those overlapping load/stores must remain 'aliasing',  so they should belong to the same scope.
>
> possible example:
>
>   // memcpy(dst, src, 23) becomes:
>   store i64 dst+0, (load i64 src+0)    // scope 0
>   store i64 dst+8, (load i64, src+8)   // scope 1, not overlapping with previous pair
>   store i64 dst+15, (load i64, src+15) // also scope 1, overlapping with previous pair

Not being able to catch that meeting. I am not sure the current SDAG would generate that. Let me check. If we did that in the current SDAG implementation, we may remove scoped AA metadata for that trailing (or maybe the heading) loads as the conservative solution before we introduce new scopes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102255