[PATCH] D124078: [MemCpyOpt] Avoid infinite loop in processMemSetMemCpyDependence (PR54983)

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: asbirlea, fhahn.
Herald added a subscriber: hiraditya.
Herald added a project: All.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This adds an additional transform to drop zero-size memcpys, also in the case where the size is only zero after instruction simplification. the motivation is the case from PR54983 where the size is non-trivially zero, and processMemSetMemCpyDependence() keeps trying to reduce the memset size by zero bytes.

I'm a bit unsure on this patch -- it's not really principled. It only works on the premise that if InstSimplify doesn't realize the size is zero, then AA also won't.

The principled approach would be to instead add a `isKnownNonZero()` guard to the `processMemSetMemCpyDependence()` transform, but I suspect that would render that optimization mostly useless (at least it breaks all the existing test coverage -- worth noting that the constant size case is also handled by DSE, so I think this transform is primarily about the dynamic size case). Though I can't say I particularly care about this transform either, so I'd be happy to go with that direction instead.


https://reviews.llvm.org/D124078

Files:
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/test/Transforms/MemCpyOpt/memcpy-zero-size.ll


Index: llvm/test/Transforms/MemCpyOpt/memcpy-zero-size.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/MemCpyOpt/memcpy-zero-size.ll
@@ -0,0 +1,26 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -memcpyopt < %s | FileCheck %s
+
+declare void @llvm.memset.p0.i64(ptr, i8, i64, i1)
+declare void @llvm.memcpy.p0.p0.i64(ptr, ptr, i64, i1)
+
+define void @zero_size(ptr %p, ptr %p2) {
+; CHECK-LABEL: @zero_size(
+; CHECK-NEXT:    ret void
+;
+  call void @llvm.memcpy.p0.p0.i64(ptr %p, ptr %p2, i64 0, i1 false)
+  ret void
+}
+
+; The memcpy size is zero in a way that is non-trivial, but understood by AA.
+define void @pr54983(ptr %p, ptr noalias %p2) {
+; CHECK-LABEL: @pr54983(
+; CHECK-NEXT:    call void @llvm.memset.p0.i64(ptr [[P:%.*]], i8 0, i64 1, i1 false)
+; CHECK-NEXT:    [[SIZE:%.*]] = shl i64 0, 0
+; CHECK-NEXT:    ret void
+;
+  call void @llvm.memset.p0.i64(ptr %p, i8 0, i64 1, i1 false)
+  %size = shl i64 0, 0
+  call void @llvm.memcpy.p0.p0.i64(ptr %p, ptr %p2, i64 %size, i1 false)
+  ret void
+}
Index: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -22,6 +22,7 @@
 #include "llvm/Analysis/AssumptionCache.h"
 #include "llvm/Analysis/CaptureTracking.h"
 #include "llvm/Analysis/GlobalsModRef.h"
+#include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/Loads.h"
 #include "llvm/Analysis/MemoryLocation.h"
 #include "llvm/Analysis/MemorySSA.h"
@@ -1390,6 +1391,15 @@
   return true;
 }
 
+static bool isZeroSize(Value *Size) {
+  if (auto *I = dyn_cast<Instruction>(Size))
+    if (auto *Res = SimplifyInstruction(I, I->getModule()->getDataLayout()))
+      Size = Res;
+  if (auto *C = dyn_cast<Constant>(Size))
+    return C->isNullValue();
+  return false;
+}
+
 /// Perform simplification of memcpy's.  If we have memcpy A
 /// which copies X to Y, and memcpy B which copies Y to Z, then we can rewrite
 /// B to be a memcpy from X to Z (or potentially a memmove, depending on
@@ -1406,6 +1416,14 @@
     return true;
   }
 
+  // If the size is zero, remove the memcpy. This also prevents infinite loops
+  // in processMemSetMemCpyDependence, which is a no-op for zero-length memcpys.
+  if (isZeroSize(M->getLength())) {
+    ++BBI;
+    eraseInstruction(M);
+    return true;
+  }
+
   // If copying from a constant, try to turn the memcpy into a memset.
   if (auto *GV = dyn_cast<GlobalVariable>(M->getSource()))
     if (GV->isConstant() && GV->hasDefinitiveInitializer())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124078.423844.patch
Type: text/x-patch
Size: 2701 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220420/1f7cb720/attachment.bin>