[PATCH] D91576: [MemCpyOpt] Bail on call slot optimization if it merges alias scopes

Wed Nov 18 17:55:28 PST 2020

modimo added a comment.

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

> Hi Modi,
>
> Following line in your input example is wrong and explains why the resulting alias info is corrupt:
>
>> call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %tmp, i8* align 8 %src, i64 1, i1 false), !alias.scope !1
>
> It should be !alias.scope !0

Thanks for verifying! Good catch, I'll update the patch with this fixed up example.

> Given that, it does not change the result of the alias analysis which indeed seems to be wrong. It seems the 'union' should only be allowed if the domains match (and maybe not just for this case ?)

Given how the problem manifests I agree that this is general to any merging of alias.scope. This is a pretty complicated area and I think I've drilled through to the ground truth.

> I would propose as solution to not 'merge' the !alias.scope in this case, but to:
>
> - keep it if it is identical on both memcpy
> - throw it away if it is not identical

That's a reasonable approach as well. I mentioned this in the summary as "We could also strip away or 'intersect' alias.scope on the final merge to still maintain correctness". I found in my specific case final codegen was not impacted by hindering this optimization and the trade-off would be less powerful AA for later phases.

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

https://reviews.llvm.org/D91576