Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MemCpyOpt] Combine alias metadatas when replacing byval arguments #70580

Merged
merged 3 commits into from
Oct 29, 2023

Conversation

dianqk
Copy link
Member

@dianqk dianqk commented Oct 29, 2023

Fixes #70578.

@dianqk dianqk requested review from lattner and khei4 October 29, 2023 03:06
@dianqk dianqk requested a review from nikic as a code owner October 29, 2023 03:06
@llvmbot
Copy link
Member

llvmbot commented Oct 29, 2023

@llvm/pr-subscribers-llvm-transforms

Author: DianQK (DianQK)

Changes

Fixes #70578.


Full diff: https://github.com/llvm/llvm-project/pull/70580.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp (+1)
  • (modified) llvm/test/Transforms/MemCpyOpt/memcpy.ll (+17)
diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
index b366ec5d4c35725..a31dedafdcbc449 100644
--- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -1864,6 +1864,7 @@ bool MemCpyOptPass::processByValArgument(CallBase &CB, unsigned ArgNo) {
                     << "  " << CB << "\n");
 
   // Otherwise we're good!  Update the byval argument.
+  combineAAMetadata(&CB, MDep);
   CB.setArgOperand(ArgNo, MDep->getSource());
   ++NumMemCpyInstr;
   return true;
diff --git a/llvm/test/Transforms/MemCpyOpt/memcpy.ll b/llvm/test/Transforms/MemCpyOpt/memcpy.ll
index f7c1c85517ffa32..7d3b4fdc1c111fe 100644
--- a/llvm/test/Transforms/MemCpyOpt/memcpy.ll
+++ b/llvm/test/Transforms/MemCpyOpt/memcpy.ll
@@ -393,6 +393,7 @@ declare void @f1(ptr nocapture sret(%struct.big))
 declare void @f2(ptr)
 
 declare void @f(ptr)
+declare void @f_byval(ptr byval(i32))
 declare void @f_full_readonly(ptr nocapture noalias readonly)
 
 define void @immut_param(ptr align 4 noalias %val) {
@@ -709,6 +710,22 @@ define void @immut_param_noalias_metadata(ptr align 4 byval(i32) %ptr) {
   ret void
 }
 
+define void @byval_param_noalias_metadata(ptr align 4 byval(i32) %ptr) {
+; CHECK-LABEL: @byval_param_noalias_metadata(
+; CHECK-NEXT:    store i32 1, ptr [[PTR:%.*]], align 4, !noalias !3
+; CHECK-NEXT:    call void @f_byval(ptr byval(i32) align 4 [[PTR]])
+; CHECK-NEXT:    ret void
+;
+  %tmp = alloca i32, align 4
+  store i32 1, ptr %ptr, !noalias !5
+  call void @llvm.memcpy.p0.p0.i64(ptr align 4 %tmp, ptr align 4 %ptr, i64 4, i1 false)
+  call void @f_byval(ptr align 4 byval(i32) %tmp), !alias.scope !5
+  ret void
+}
+
 !0 = !{!0}
 !1 = !{!1, !0}
 !2 = !{!1}
+!3 = !{!3}
+!4 = !{!4, !3}
+!5 = !{!4}

Copy link
Contributor

@khei4 khei4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Seems reasonable to me:)

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dianqk dianqk merged commit 0c4f326 into llvm:main Oct 29, 2023
@dianqk dianqk deleted the memcpyopt-byval-alias branch October 29, 2023 08:08
tru pushed a commit that referenced this pull request Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MemCpyOpt] The store instruction should not be removed by DSE.
4 participants