[PATCH] D119929: [MemCpyOpt] Check uses of found Clobber in writtenBetween.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 410296.
fhahn added a comment.



In D119929#3333385 <https://reviews.llvm.org/D119929#3333385>, @nikic wrote:

>> I think we also need to do that for MemoryDefs, if End doesn't clobber Loc, because then the defining access could be optimized by skipping any accesses to Loc unless I am missing something.
>
> Unlike MemoryUses, MemoryDefs have a separate defining access and optimized access. The defining access always points to the dominating MemoryDef/MemoryPhi, regardless of whether it clobbers any locations of the access. The optimized access does take clobbers into account.

Ah right, the walker always only uses the optimized accesses for MemoryUses!

> If you are looking for a quick fix here, then the way to do it is to use the code as-is for MemoryDefs and only limit to the single-BB walk for MemoryUses. That should fix the issue with readonly byval calls, while keeping the much more important memcpy-memcpy dependence optimization working as before.

Done in the latest version, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119929/new/

https://reviews.llvm.org/D119929

Files:
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/test/Transforms/MemCpyOpt/memcpy-byval-forwarding-clobbers.ll


Index: llvm/test/Transforms/MemCpyOpt/memcpy-byval-forwarding-clobbers.ll
===================================================================
--- llvm/test/Transforms/MemCpyOpt/memcpy-byval-forwarding-clobbers.ll
+++ llvm/test/Transforms/MemCpyOpt/memcpy-byval-forwarding-clobbers.ll
@@ -13,7 +13,6 @@
 
 ; %a.2's lifetime ends before the call to @check. Cannot replace
 ; %a.1 with %a.2 in the call to @check.
-; FIXME: Find lifetime.end, prevent optimization.
 define i1 @alloca_forwarding_lifetime_end_clobber() {
 ; CHECK-LABEL: @alloca_forwarding_lifetime_end_clobber(
 ; CHECK-NEXT:  entry:
@@ -26,7 +25,7 @@
 ; CHECK-NEXT:    store i8 0, i8* [[BC_A_2]], align 1
 ; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* [[BC_A_1]], i8* [[BC_A_2]], i64 8, i1 false)
 ; CHECK-NEXT:    call void @llvm.lifetime.end.p0i8(i64 8, i8* [[BC_A_2]])
-; CHECK-NEXT:    [[CALL:%.*]] = call i1 @check(i64* byval(i64) align 8 [[A_2]])
+; CHECK-NEXT:    [[CALL:%.*]] = call i1 @check(i64* byval(i64) align 8 [[A_1]])
 ; CHECK-NEXT:    ret i1 [[CALL]]
 ;
 entry:
@@ -46,7 +45,6 @@
 
 ; There is a call clobbering %a.2 before the call to @check. Cannot replace
 ; %a.1 with %a.2 in the call to @check.
-; FIXME: Find clobber, prevent optimization.
 define i1 @alloca_forwarding_call_clobber() {
 ; CHECK-LABEL: @alloca_forwarding_call_clobber(
 ; CHECK-NEXT:  entry:
@@ -59,7 +57,7 @@
 ; CHECK-NEXT:    store i8 0, i8* [[BC_A_2]], align 1
 ; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* [[BC_A_1]], i8* [[BC_A_2]], i64 8, i1 false)
 ; CHECK-NEXT:    call void @clobber(i8* [[BC_A_2]])
-; CHECK-NEXT:    [[CALL:%.*]] = call i1 @check(i64* byval(i64) align 8 [[A_2]])
+; CHECK-NEXT:    [[CALL:%.*]] = call i1 @check(i64* byval(i64) align 8 [[A_1]])
 ; CHECK-NEXT:    ret i1 [[CALL]]
 ;
 entry:
Index: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -352,13 +352,32 @@
 
 // Check for mod of Loc between Start and End, excluding both boundaries.
 // Start and End can be in different blocks.
-static bool writtenBetween(MemorySSA *MSSA, MemoryLocation Loc,
-                           const MemoryUseOrDef *Start,
+static bool writtenBetween(MemorySSA *MSSA, AliasAnalysis &AA,
+                           MemoryLocation Loc, const MemoryUseOrDef *Start,
                            const MemoryUseOrDef *End) {
   // TODO: Only walk until we hit Start.
   MemoryAccess *Clobber = MSSA->getWalker()->getClobberingMemoryAccess(
       End->getDefiningAccess(), Loc);
-  return !MSSA->dominates(Clobber, Start);
+
+  if (!MSSA->dominates(Clobber, Start))
+    return true;
+
+  if (isa<MemoryUse>(End)) {
+    // For MemoryUses, getClobberingMemoryAccess may skip non-clobbering writes.
+    // Manually check read accesses between Start and End, if they are in the
+    // same block, for clobbers. Otherwise assume Loc is clobbered.
+    return Start->getBlock() != End->getBlock() ||
+           any_of(
+               make_range(std::next(Start->getIterator()), End->getIterator()),
+               [&AA, Loc](const MemoryAccess &Acc) {
+                 if (isa<MemoryUse>(&Acc))
+                   return false;
+                 Instruction *AccInst =
+                     cast<MemoryUseOrDef>(&Acc)->getMemoryInst();
+                 return isModSet(AA.getModRefInfo(AccInst, Loc));
+               });
+  }
+  return false;
 }
 
 /// When scanning forward over instructions, we look for some other patterns to
@@ -1118,7 +1137,7 @@
   // then we could still perform the xform by moving M up to the first memcpy.
   // TODO: It would be sufficient to check the MDep source up to the memcpy
   // size of M, rather than MDep.
-  if (writtenBetween(MSSA, MemoryLocation::getForSource(MDep),
+  if (writtenBetween(MSSA, *AA, MemoryLocation::getForSource(MDep),
                      MSSA->getMemoryAccess(MDep), MSSA->getMemoryAccess(M)))
     return false;
 
@@ -1557,7 +1576,7 @@
   //    *b = 42;
   //    foo(*a)
   // It would be invalid to transform the second memcpy into foo(*b).
-  if (writtenBetween(MSSA, MemoryLocation::getForSource(MDep),
+  if (writtenBetween(MSSA, *AA, MemoryLocation::getForSource(MDep),
                      MSSA->getMemoryAccess(MDep), MSSA->getMemoryAccess(&CB)))
     return false;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D119929.410296.patch
Type: text/x-patch
Size: 4400 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220221/830884d7/attachment.bin>