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] Use dyn_cast to fix assertion failure in processMemCpyMemCpyDependence #98686

Merged
merged 2 commits into from
Jul 12, 2024

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Jul 12, 2024

Fixes #98675.

@llvmbot
Copy link
Collaborator

llvmbot commented Jul 12, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

Fixes #98675.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp (+3-2)
  • (modified) llvm/test/Transforms/MemCpyOpt/memcpy-memcpy-offset.ll (+18)
diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
index 1c65219585e5a..c542084c1ad2f 100644
--- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -1188,8 +1188,9 @@ bool MemCpyOptPass::processMemCpyMemCpyDependence(MemCpyInst *M,
     if (MDestOffset == MForwardOffset)
       CopySource = M->getDest();
     else {
-      NewCopySource = cast<Instruction>(Builder.CreateInBoundsPtrAdd(
-          CopySource, Builder.getInt64(MForwardOffset)));
+      NewCopySource = GetElementPtrInst::CreateInBounds(
+          Builder.getInt8Ty(), CopySource, Builder.getInt64(MForwardOffset), "",
+          Builder.GetInsertPoint());
       CopySource = NewCopySource;
     }
     // We need to update `MCopyLoc` if an offset exists.
diff --git a/llvm/test/Transforms/MemCpyOpt/memcpy-memcpy-offset.ll b/llvm/test/Transforms/MemCpyOpt/memcpy-memcpy-offset.ll
index 6abb0da827990..455a06045ab8d 100644
--- a/llvm/test/Transforms/MemCpyOpt/memcpy-memcpy-offset.ll
+++ b/llvm/test/Transforms/MemCpyOpt/memcpy-memcpy-offset.ll
@@ -197,6 +197,24 @@ define void @do_not_forward_offset_and_store(ptr %src, ptr %dest) {
   ret void
 }
 
+; Make sure we don't crash when the copy source is a constant.
+@buf = external global [32 x i8]
+
+define void @pr98675(ptr noalias %p1, ptr noalias %p2) {
+; CHECK-LABEL: define void @pr98675(
+; CHECK-SAME: ptr noalias [[P1:%.*]], ptr noalias [[P2:%.*]]) {
+; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr [[P1]], ptr @buf, i64 26, i1 false)
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i8, ptr [[P1]], i64 10
+; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds i8, ptr @buf, i64 10
+; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr [[P2]], ptr [[TMP1]], i64 1, i1 false)
+; CHECK-NEXT:    ret void
+;
+  call void @llvm.memcpy.p0.p0.i64(ptr %p1, ptr @buf, i64 26, i1 false)
+  %gep = getelementptr i8, ptr %p1, i64 10
+  call void @llvm.memmove.p0.p0.i64(ptr %p2, ptr %gep, i64 1, i1 false)
+  ret void
+}
+
 declare void @use(ptr)
 
 declare void @llvm.memcpy.p0.p0.i64(ptr nocapture, ptr nocapture, i64, i1)

@dtcxzyw dtcxzyw linked an issue Jul 12, 2024 that may be closed by this pull request
@dtcxzyw dtcxzyw changed the title [MemCpyOpt] Always create gep instructions in processMemCpyMemCpyDependence [MemCpyOpt] Use dyn_cast to fix assertion failure in processMemCpyMemCpyDependence Jul 12, 2024
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

@dtcxzyw dtcxzyw merged commit 99685a5 into llvm:main Jul 12, 2024
4 of 6 checks passed
@dtcxzyw dtcxzyw deleted the fix-pr98675 branch July 12, 2024 20:27
@DianQK
Copy link
Member

DianQK commented Jul 12, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants