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

[InstCombine] Dont throw away noalias/alias scope metadata when inlining memcpys #74805

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

gbaraldi
Copy link
Contributor

@gbaraldi gbaraldi commented Dec 8, 2023

This was found in julia when we changed some operations from explicit loads + stores to memcpys. While applying it to both the src and the dest seems weird, thats what we do for normal TBAA.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 8, 2023

@llvm/pr-subscribers-llvm-transforms

Author: Gabriel Baraldi (gbaraldi)

Changes

This was found in julia when we changed some operations from explicit loads + stores to memcpys. While applying it to both the src and the dest seems weird, thats what we do for normal TBAA.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+7-9)
  • (modified) llvm/test/Transforms/InstCombine/memcpy-to-load.ll (+16)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index a991f0906052a3..110d5854f62e69 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -172,10 +172,10 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
 
   // If the memcpy has metadata describing the members, see if we can get the
   // TBAA tag describing our copy.
-  MDNode *CopyMD = nullptr;
-  if (MDNode *M = MI->getMetadata(LLVMContext::MD_tbaa)) {
-    CopyMD = M;
-  } else if (MDNode *M = MI->getMetadata(LLVMContext::MD_tbaa_struct)) {
+  AAMDNodes AACopyMD = MI->getAAMetadata();
+
+  if (MDNode *M = AACopyMD.TBAAStruct) {
+    AACopyMD.TBAAStruct = nullptr;
     if (M->getNumOperands() == 3 && M->getOperand(0) &&
         mdconst::hasa<ConstantInt>(M->getOperand(0)) &&
         mdconst::extract<ConstantInt>(M->getOperand(0))->isZero() &&
@@ -184,7 +184,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
         mdconst::extract<ConstantInt>(M->getOperand(1))->getValue() ==
         Size &&
         M->getOperand(2) && isa<MDNode>(M->getOperand(2)))
-      CopyMD = cast<MDNode>(M->getOperand(2));
+      AACopyMD.TBAA = cast<MDNode>(M->getOperand(2));
   }
 
   Value *Src = MI->getArgOperand(1);
@@ -192,8 +192,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
   LoadInst *L = Builder.CreateLoad(IntType, Src);
   // Alignment from the mem intrinsic will be better, so use it.
   L->setAlignment(*CopySrcAlign);
-  if (CopyMD)
-    L->setMetadata(LLVMContext::MD_tbaa, CopyMD);
+  L->setAAMetadata(AACopyMD);
   MDNode *LoopMemParallelMD =
     MI->getMetadata(LLVMContext::MD_mem_parallel_loop_access);
   if (LoopMemParallelMD)
@@ -205,8 +204,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
   StoreInst *S = Builder.CreateStore(L, Dest);
   // Alignment from the mem intrinsic will be better, so use it.
   S->setAlignment(*CopyDstAlign);
-  if (CopyMD)
-    S->setMetadata(LLVMContext::MD_tbaa, CopyMD);
+  S->setAAMetadata(AACopyMD);
   if (LoopMemParallelMD)
     S->setMetadata(LLVMContext::MD_mem_parallel_loop_access, LoopMemParallelMD);
   if (AccessGroupMD)
diff --git a/llvm/test/Transforms/InstCombine/memcpy-to-load.ll b/llvm/test/Transforms/InstCombine/memcpy-to-load.ll
index 2f13394b5e477d..f8e8ab85f935af 100644
--- a/llvm/test/Transforms/InstCombine/memcpy-to-load.ll
+++ b/llvm/test/Transforms/InstCombine/memcpy-to-load.ll
@@ -79,3 +79,19 @@ define void @copy_16_bytes(ptr %d, ptr %s) {
   ret void
 }
 
+define void @copy_8_bytes_noalias(ptr %d, ptr %s) {
+; CHECK-LABEL: @copy_8_bytes_noalias(
+; CHECK-NEXT:    [[TMP1:%.*]] = load i64, ptr [[S:%.*]], align 1, !alias.scope [[META0:![0-9]+]], !noalias [[META3:![0-9]+]]
+; CHECK-NEXT:    store i64 [[TMP1]], ptr [[D:%.*]], align 1, !alias.scope [[META0]], !noalias [[META3]]
+; CHECK-NEXT:    ret void
+;
+  call void @llvm.memcpy.p0.p0.i32(ptr %d, ptr %s, i32 8, i1 false), !alias.scope !4, !noalias !5
+  ret void
+}
+
+!0 = distinct !{!0, !"The domain"}
+!1 = distinct !{!1}
+!2 = !{!2, !0}
+!3 = !{!3, !1}
+!4 = !{!2}
+!5 = !{!3}

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

gbaraldi added a commit to JuliaLang/llvm-project that referenced this pull request Dec 8, 2023
vchuravy pushed a commit to JuliaLang/llvm-project that referenced this pull request Dec 8, 2023
@gbaraldi
Copy link
Contributor Author

gbaraldi commented Jan 4, 2024

bump!

@nikic nikic merged commit a87fa7f into llvm:main Jan 4, 2024
6 checks passed
gbaraldi added a commit to gbaraldi/llvm-project that referenced this pull request Apr 22, 2024
…ing memcpys (llvm#74805)

This was found in julia when we changed some operations from explicit
loads + stores to memcpys. While applying it to both the src and the
dest seems weird, thats what we do for normal TBAA.
gbaraldi added a commit to JuliaLang/llvm-project that referenced this pull request Apr 22, 2024
…ing memcpys (llvm#74805)

This was found in julia when we changed some operations from explicit
loads + stores to memcpys. While applying it to both the src and the
dest seems weird, thats what we do for normal TBAA.
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.

None yet

4 participants