Skip to content

[MemCpyOpt] Avoid infinite loops in MemCpyOptPass::processMemCpyMemCpyDependence #103218

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 22, 2024
Merged

[MemCpyOpt] Avoid infinite loops in MemCpyOptPass::processMemCpyMemCpyDependence #103218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7bc99d3
[MemCpyOpt] Avoid infinite loops in `MemCpyOptPass::processMemCpyMemC…
dtcxzyw Aug 13, 2024
bd8d6dd
[MemCpyOpt] Add new tests without constantexpr
dtcxzyw Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1193,6 +1193,10 @@ bool MemCpyOptPass::processMemCpyMemCpyDependence(MemCpyInst *M,
CopySourceAlign = commonAlignment(*CopySourceAlign, MForwardOffset);
}

// Avoid infinite loops
if (BAA.isMustAlias(M->getSource(), CopySource))
Copy link
Contributor

Choose a reason for hiding this comment

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

There already is a M->getSource() == MDep->getSource() check above. Can we change that one?

Also, do you think it would make sense to also remove the no-op memcpy, if we already have to check for them anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to change this check into BAA.isMustAlias(M->getSource(), MDep->getSource()). But it doesn't work.

Also, do you think it would make sense to also remove the no-op memcpy, if we already have to check for them anyway?

Yeah, it's definitely a better approach. But before doing this, we should prove that ptr getelementptr inbounds (i8, ptr @g2, i64 16) and ptr getelementptr inbounds nuw (i8, ptr @g2, i64 16) must alias.

Copy link
Member

@dianqk dianqk May 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I tried to change this check into BAA.isMustAlias(M->getSource(), MDep->getSource()). But it doesn't work.

This appears to should be BAA.isMustAlias(MDep->getDest(), MDep->getSource()) #140376.

Also, do you think it would make sense to also remove the no-op memcpy, if we already have to check for them anyway?

Yeah, it's definitely a better approach. But before doing this, we should prove that ptr getelementptr inbounds (i8, ptr @g2, i64 16) and ptr getelementptr inbounds nuw (i8, ptr @g2, i64 16) must alias.

This should already must alias, and it seems we can handle it before processMemCpyMemCpyDependence.

return false;

// Verify that the copied-from memory doesn't change in between the two
// transfers. For example, in:
// memcpy(a <- b)
Expand Down
39 changes: 39 additions & 0 deletions llvm/test/Transforms/MemCpyOpt/pr102994.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
; RUN: opt -S -passes=memcpyopt < %s | FileCheck %s

@g1 = external global i8
@g2 = external global [64 x i8]
@g3 = global i8 0, align 1

define void @func() {
; CHECK-LABEL: define void @func() {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr getelementptr inbounds (i8, ptr @g2, i64 16), ptr getelementptr inbounds nuw (i8, ptr @g2, i64 16), i64 20, i1 false)
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr @g1, ptr getelementptr inbounds (i8, ptr @g2, i64 24), i64 1, i1 false)
; CHECK-NEXT: ret void
;
entry:
call void @llvm.memcpy.p0.p0.i64(ptr getelementptr inbounds (i8, ptr @g2, i64 16), ptr getelementptr inbounds nuw (i8, ptr @g2, i64 16), i64 20, i1 false)
call void @llvm.memcpy.p0.p0.i64(ptr @g1, ptr getelementptr inbounds (i8, ptr @g2, i64 24), i64 1, i1 false)
ret void
}

define void @func2(ptr %p) {
; CHECK-LABEL: define void @func2(
; CHECK-SAME: ptr [[P:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: [[GEP1:%.*]] = getelementptr i8, ptr [[P]], i64 32
; CHECK-NEXT: [[GEP2:%.*]] = getelementptr i8, ptr [[P]], i64 34
; CHECK-NEXT: [[GEP3:%.*]] = getelementptr i8, ptr [[P]], i64 32
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr [[GEP1]], ptr [[GEP3]], i64 32, i1 false)
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr @g3, ptr [[GEP2]], i64 1, i1 false)
; CHECK-NEXT: ret void
;
entry:
%gep1 = getelementptr i8, ptr %p, i64 32
%gep2 = getelementptr i8, ptr %p, i64 34
%gep3 = getelementptr i8, ptr %p, i64 32
call void @llvm.memcpy.p0.p0.i64(ptr %gep1, ptr %gep3, i64 32, i1 false)
call void @llvm.memcpy.p0.p0.i64(ptr @g3, ptr %gep2, i64 1, i1 false)
ret void
}
Loading