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] No need to create memcpy(a <- a) #98321

Merged
merged 4 commits into from
Jul 11, 2024

Conversation

DianQK
Copy link
Member

@DianQK DianQK commented Jul 10, 2024

When forwarding memcpy, we don't need to create memcpy(a, a).

I fixed a related test case at 62b3e68.

@DianQK DianQK requested a review from dtcxzyw July 10, 2024 13:56
@DianQK DianQK requested a review from nikic as a code owner July 10, 2024 13:56
@llvmbot
Copy link
Member

llvmbot commented Jul 10, 2024

@llvm/pr-subscribers-llvm-transforms

Author: DianQK (DianQK)

Changes

When forwarding memcpy, we don't need to create memcpy(a, a).


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp (+8)
  • (modified) llvm/test/Transforms/MemCpyOpt/memcpy.ll (+50)
diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
index b9efd9aaa28c5..1a4b3657cedae 100644
--- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -1161,6 +1161,14 @@ bool MemCpyOptPass::processMemCpyMemCpyDependence(MemCpyInst *M,
                      MSSA->getMemoryAccess(MDep), MSSA->getMemoryAccess(M)))
     return false;
 
+  // No need to create `memcpy(a <- a)`.
+  if (BAA.isMustAlias(M->getDest(), MDep->getRawSource())) {
+    // Remove the instruction we're replacing.
+    eraseInstruction(M);
+    ++NumMemCpyInstr;
+    return true;
+  }
+
   // If the dest of the second might alias the source of the first, then the
   // source and dest might overlap. In addition, if the source of the first
   // points to constant memory, they won't overlap by definition. Otherwise, we
diff --git a/llvm/test/Transforms/MemCpyOpt/memcpy.ll b/llvm/test/Transforms/MemCpyOpt/memcpy.ll
index 48698f25174e7..a19cfe6e75b70 100644
--- a/llvm/test/Transforms/MemCpyOpt/memcpy.ll
+++ b/llvm/test/Transforms/MemCpyOpt/memcpy.ll
@@ -139,6 +139,56 @@ define void @test6_memcpy(ptr %src, ptr %dest) nounwind {
   ret void
 }
 
+; FIXME: When forwarding to memcpy(arg+1, arg+1), we don't need to create this memcpy.
+define void @test6_memcpy_forward_back(ptr %arg) nounwind {
+; CHECK-LABEL: @test6_memcpy_forward_back(
+; CHECK-NEXT:    [[DEST:%.*]] = getelementptr inbounds i8, ptr [[ARG:%.*]], i64 1
+; CHECK-NEXT:    ret void
+;
+  %tmp = alloca [16 x i8], align 1
+  %src = getelementptr inbounds i8, ptr %arg, i64 1
+  %dest = getelementptr inbounds i8, ptr %arg, i64 1
+  call void @llvm.memcpy.inline.p0.p0.i32(ptr align 1 %tmp, ptr align 1 %src, i32 16, i1 false)
+  call void @llvm.memcpy.inline.p0.p0.i32(ptr align 1 %dest, ptr align 1 %tmp, i32 16, i1 false)
+  ret void
+}
+
+; We have to retain this `memcpy(arg+2, arg+1)` forwarding.
+define void @test6_memcpy_forward_not_back(ptr %arg) nounwind {
+; CHECK-LABEL: @test6_memcpy_forward_not_back(
+; CHECK-NEXT:    [[TMP:%.*]] = alloca [16 x i8], align 1
+; CHECK-NEXT:    [[SRC:%.*]] = getelementptr inbounds i8, ptr [[ARG:%.*]], i64 1
+; CHECK-NEXT:    [[DEST:%.*]] = getelementptr inbounds i8, ptr [[ARG]], i64 2
+; CHECK-NEXT:    call void @llvm.memcpy.inline.p0.p0.i32(ptr align 1 [[TMP]], ptr align 1 [[SRC]], i32 16, i1 false)
+; CHECK-NEXT:    call void @llvm.memcpy.inline.p0.p0.i32(ptr align 1 [[DEST]], ptr align 1 [[TMP]], i32 16, i1 false)
+; CHECK-NEXT:    ret void
+;
+  %tmp = alloca [16 x i8], align 1
+  %src = getelementptr inbounds i8, ptr %arg, i64 1
+  %dest = getelementptr inbounds i8, ptr %arg, i64 2
+  call void @llvm.memcpy.inline.p0.p0.i32(ptr align 1 %tmp, ptr align 1 %src, i32 16, i1 false)
+  call void @llvm.memcpy.inline.p0.p0.i32(ptr align 1 %dest, ptr align 1 %tmp, i32 16, i1 false)
+  ret void
+}
+
+; There is data writing between the two memcpy operations.
+define void @test6_memcpy_forward_back_failed(ptr %arg) nounwind {
+; CHECK-LABEL: @test6_memcpy_forward_back_failed(
+; CHECK-NEXT:    [[TMP:%.*]] = alloca [16 x i8], align 1
+; CHECK-NEXT:    [[SRC:%.*]] = getelementptr inbounds i8, ptr [[ARG:%.*]], i64 1
+; CHECK-NEXT:    [[DEST:%.*]] = getelementptr inbounds i8, ptr [[ARG]], i64 1
+; CHECK-NEXT:    call void @llvm.memcpy.inline.p0.p0.i32(ptr align 1 [[TMP]], ptr align 1 [[SRC]], i32 16, i1 false)
+; CHECK-NEXT:    call void @llvm.memcpy.inline.p0.p0.i32(ptr align 1 [[DEST]], ptr align 1 [[TMP]], i32 16, i1 false)
+; CHECK-NEXT:    ret void
+;
+  %tmp = alloca [16 x i8], align 1
+  %src = getelementptr inbounds i8, ptr %arg, i64 1
+  %dest = getelementptr inbounds i8, ptr %arg, i64 1
+  call void @llvm.memcpy.inline.p0.p0.i32(ptr align 1 %tmp, ptr align 1 %src, i32 16, i1 false)
+  store i8 1, ptr %dest, align 1
+  call void @llvm.memcpy.inline.p0.p0.i32(ptr align 1 %dest, ptr align 1 %tmp, i32 16, i1 false)
+  ret void
+}
 
 @x = external global %0
 

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

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 117cc4a into llvm:main Jul 11, 2024
7 checks passed
@DianQK DianQK deleted the memcpy-forward-back branch July 11, 2024 11:54
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 11, 2024

LLVM Buildbot has detected a new failure on builder sanitizer-ppc64be-linux running on ppc64be-sanitizer while building llvm at step 2 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/109/builds/861

Here is the relevant piece of the build log for the reference:

Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure)
...
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-with-call-Test/60/281 (1571 of 2429)
PASS: ScudoStandalone-Unit :: ./ScudoUnitTest-powerpc64-Test/66/274 (1572 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/109/281 (1573 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-with-call-Test/59/281 (1574 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/141/281 (1575 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-with-call-Test/23/281 (1576 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/49/281 (1577 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/15/281 (1578 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/40/281 (1579 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/125/281 (1580 of 2429)
FAIL: ThreadSanitizer-powerpc64 :: signal_reset.cpp (1581 of 2429)
******************** TEST 'ThreadSanitizer-powerpc64 :: signal_reset.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/./bin/clang  --driver-mode=g++ -fsanitize=thread -Wall  -m64   -gline-tables-only -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/../ -std=c++11 -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/../ -nostdinc++ -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/lib/tsan/libcxx_tsan_powerpc64/include/c++/v1 -O1 /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp -o /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_reset.cpp.tmp &&  /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_reset.cpp.tmp 2>&1 | FileCheck /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp
+ /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/./bin/clang --driver-mode=g++ -fsanitize=thread -Wall -m64 -gline-tables-only -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/../ -std=c++11 -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/../ -nostdinc++ -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/lib/tsan/libcxx_tsan_powerpc64/include/c++/v1 -O1 /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp -o /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_reset.cpp.tmp
+ /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_reset.cpp.tmp
+ FileCheck /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp
/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp:73:15: error: CHECK-NOT: excluded string found in input
// CHECK-NOT: WARNING: ThreadSanitizer:
              ^
<stdin>:2:1: note: found here
WARNING: ThreadSanitizer: signal handler spoils errno (pid=85219)
^~~~~~~~~~~~~~~~~~~~~~~~~

Input file: <stdin>
Check file: /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp

-dump-input=help explains the following input dump.

Input was:
<<<<<<
        1: ================== 
        2: WARNING: ThreadSanitizer: signal handler spoils errno (pid=85219) 
not:73     !~~~~~~~~~~~~~~~~~~~~~~~~                                          error: no match expected
        3:  Signal 27 handler invoked at: 
        4:  #0 _DYNAMIC <null> (signal_reset.cpp.tmp+0x16fe78) 
        5:  
        6: SUMMARY: ThreadSanitizer: signal handler spoils errno (/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_reset.cpp.tmp+0x16fe78) in _DYNAMIC 
        7: ================== 
        8: DONE 
        9: ThreadSanitizer: reported 1 warnings 
>>>>>>

--

********************
Step 11 (test compiler-rt debug) failure: test compiler-rt debug (failure)
...
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-with-call-Test/60/281 (1571 of 2429)
PASS: ScudoStandalone-Unit :: ./ScudoUnitTest-powerpc64-Test/66/274 (1572 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/109/281 (1573 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-with-call-Test/59/281 (1574 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/141/281 (1575 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-with-call-Test/23/281 (1576 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/49/281 (1577 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/15/281 (1578 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/40/281 (1579 of 2429)
PASS: MemorySanitizer-Unit :: ./Msan-powerpc64-Test/125/281 (1580 of 2429)
FAIL: ThreadSanitizer-powerpc64 :: signal_reset.cpp (1581 of 2429)
******************** TEST 'ThreadSanitizer-powerpc64 :: signal_reset.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/./bin/clang  --driver-mode=g++ -fsanitize=thread -Wall  -m64   -gline-tables-only -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/../ -std=c++11 -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/../ -nostdinc++ -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/lib/tsan/libcxx_tsan_powerpc64/include/c++/v1 -O1 /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp -o /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_reset.cpp.tmp &&  /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_reset.cpp.tmp 2>&1 | FileCheck /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp
+ /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/./bin/clang --driver-mode=g++ -fsanitize=thread -Wall -m64 -gline-tables-only -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/../ -std=c++11 -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/../ -nostdinc++ -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/lib/tsan/libcxx_tsan_powerpc64/include/c++/v1 -O1 /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp -o /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_reset.cpp.tmp
+ /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_reset.cpp.tmp
+ FileCheck /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp
/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp:73:15: error: CHECK-NOT: excluded string found in input
// CHECK-NOT: WARNING: ThreadSanitizer:
              ^
<stdin>:2:1: note: found here
WARNING: ThreadSanitizer: signal handler spoils errno (pid=85219)
^~~~~~~~~~~~~~~~~~~~~~~~~

Input file: <stdin>
Check file: /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_reset.cpp

-dump-input=help explains the following input dump.

Input was:
<<<<<<
        1: ================== 
        2: WARNING: ThreadSanitizer: signal handler spoils errno (pid=85219) 
not:73     !~~~~~~~~~~~~~~~~~~~~~~~~                                          error: no match expected
        3:  Signal 27 handler invoked at: 
        4:  #0 _DYNAMIC <null> (signal_reset.cpp.tmp+0x16fe78) 
        5:  
        6: SUMMARY: ThreadSanitizer: signal handler spoils errno (/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_reset.cpp.tmp+0x16fe78) in _DYNAMIC 
        7: ================== 
        8: DONE 
        9: ThreadSanitizer: reported 1 warnings 
>>>>>>

--

********************

aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
When forwarding `memcpy`, we don't need to create `memcpy(a, a)`.
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.

5 participants