[PATCH] D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization

Mon Nov 23 11:44:02 PST 2020

jeroen.dobbelaere added a comment.

In D91576#2411078 <https://reviews.llvm.org/D91576#2411078>, @nikic wrote:

> Unless there's something that makes this issue specific to the call slot optimization, wouldn't it be better to adjust the logic in `MDNode::getMostGenericAliasScope()` to ensure that any code performing alias scope merging is covered?

Yes, after thinking some more about it, that is probably the best solution. This also corresponds to what the Full Restrict patches are doing when merging ptr_provenance operands. The merging/handling of different domains should only be done during inlining and that seems to be done explicitly in InlineFunction.cpp. It is for this use case that the langref explains the behavior of alias.scope/noalias wrt handling of aliasing.

================
Comment at: llvm/test/Transforms/MemCpyOpt/callslot_badaa.ll:20
+; Note we're matching the full line and looking for the lack of !alias.scope here
+; CHECK:   call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %dst, i8* align 8 %src, i64 1, i1 false)
+  call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %src), !noalias !3
----------------
It would be nice if we could check that the !alias.scope really has been omitted.

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

https://reviews.llvm.org/D91576