[PATCH] D66060: [MemCpyOpt] Fixing Incorrect Code Motion while Handling Aggregate Type Values

Min-Yih Hsu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 10 21:42:48 PDT 2019


myhsu created this revision.
myhsu added reviewers: eugenis, pcc, dblaikie.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

When MemCpyOpt is handling aggregate type values, if an instruction (let's call it P) between the targeting load (L) and store (S) clobbers the source pointer of L, it will try to hoist S before P. This process will also hoist S's data dependency instructions.

However, the current implementation has a bug that if one of S's dependency instructions is //also// a user of P, MemCpyOpt will not prevent it from being hoisted above P and cause a use-before-define error. Please refer to the newly added test file (i.e. `aggregate-type-crash.ll`) for more details.


Repository:
  rL LLVM

https://reviews.llvm.org/D66060

Files:
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/test/Transforms/MemCpyOpt/aggregate-type-crash.ll


Index: llvm/test/Transforms/MemCpyOpt/aggregate-type-crash.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/MemCpyOpt/aggregate-type-crash.ll
@@ -0,0 +1,30 @@
+; RUN: opt -memcpyopt -S -o - < %s | FileCheck %s
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.14.0"
+
+%my_struct = type { i8, i32 }
+
+; Function Attrs: inaccessiblemem_or_argmemonly
+declare noalias i8* @my_malloc(%my_struct*) #0
+
+define void @my_func(%my_struct* %0) {
+entry:
+; CHECK: entry:
+  %1 = load %my_struct, %my_struct* %0
+  %2 = call i8* @my_malloc(%my_struct* %0)
+  %3 = bitcast i8* %2 to %my_struct*
+  store %my_struct %1, %my_struct* %3
+; CHECK-NOT: call void @llvm.memcpy.{{.*}}.{{.*}}.{{.*}}
+  ret void
+}
+
+attributes #0 = { inaccessiblemem_or_argmemonly }
+
+!llvm.module.flags = !{!0, !1, !2}
+!llvm.ident = !{!3}
+
+!0 = !{i32 2, !"SDK Version", [2 x i32] [i32 10, i32 14]}
+!1 = !{i32 1, !"wchar_size", i32 4}
+!2 = !{i32 7, !"PIC Level", i32 2}
+!3 = !{!"Apple LLVM version 10.0.1 (clang-1001.0.46.4)"}
Index: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -597,9 +597,13 @@
 
     ToLift.push_back(C);
     for (unsigned k = 0, e = C->getNumOperands(); k != e; ++k)
-      if (auto *A = dyn_cast<Instruction>(C->getOperand(k)))
-        if (A->getParent() == SI->getParent())
+      if (auto *A = dyn_cast<Instruction>(C->getOperand(k))) {
+        if (A->getParent() == SI->getParent()) {
+          // Cannot hoist user of P above P
+          if(A == P) return false;
           Args.insert(A);
+        }
+      }
   }
 
   // We made it, we need to lift


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66060.214548.patch
Type: text/x-patch
Size: 1847 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190811/ae766494/attachment.bin>


More information about the llvm-commits mailing list