[llvm] r330635 - [MemCpyOpt] Skip optimizing basic blocks not reachable from entry

[Bjorn Pettersson via llvm-commits]

Author: bjope
Date: Mon Apr 23 12:55:04 2018
New Revision: 330635

URL: http://llvm.org/viewvc/llvm-project?rev=330635&view=rev
Log:
[MemCpyOpt] Skip optimizing basic blocks not reachable from entry

Summary:
Skip basic blocks not reachable from the entry node
in MemCpyOptPass::iterateOnFunction.

Code that is unreachable may have properties that do not exist
for reachable code (an instruction in a basic block can for
example be dominated by a later instruction in the same basic
block, for example if there is a single block loop).
MemCpyOptPass::processStore is only safe to use for reachable
basic blocks, since it may iterate past the basic block
beginning when used for unreachable blocks. By simply skipping
to optimize unreachable basic blocks we can avoid asserts such
as "Assertion `!NodePtr->isKnownSentinel()' failed."
in MemCpyOptPass::processStore.

The problem was detected by fuzz tests.

Reviewers: eli.friedman, dneilson, efriedma

Reviewed By: efriedma

Subscribers: efriedma, llvm-commits

Differential Revision: https://reviews.llvm.org/D45889

Added:
    llvm/trunk/test/Transforms/MemCpyOpt/process_store.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp?rev=330635&r1=330634&r2=330635&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp Mon Apr 23 12:55:04 2018
@@ -1391,10 +1391,19 @@ bool MemCpyOptPass::processByValArgument
 bool MemCpyOptPass::iterateOnFunction(Function &F) {
   bool MadeChange = false;
 
+  DominatorTree &DT = LookupDomTree();
+
   // Walk all instruction in the function.
   for (BasicBlock &BB : F) {
+    // Skip unreachable blocks. For example processStore assumes that an
+    // instruction in a BB can't be dominated by a later instruction in the
+    // same BB (which is a scenario that can happen for an unreachable BB that
+    // has itself as a predecessor).
+    if (!DT.isReachableFromEntry(&BB))
+      continue;
+
     for (BasicBlock::iterator BI = BB.begin(), BE = BB.end(); BI != BE;) {
-      // Avoid invalidating the iterator.
+        // Avoid invalidating the iterator.
       Instruction *I = &*BI++;
 
       bool RepeatInstruction = false;

Added: llvm/trunk/test/Transforms/MemCpyOpt/process_store.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MemCpyOpt/process_store.ll?rev=330635&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/MemCpyOpt/process_store.ll (added)
+++ llvm/trunk/test/Transforms/MemCpyOpt/process_store.ll Mon Apr 23 12:55:04 2018
@@ -0,0 +1,39 @@
+; RUN: opt < %s -memcpyopt -disable-output
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at b = common dso_local local_unnamed_addr global i32 0, align 4
+ at a = common dso_local local_unnamed_addr global i32 0, align 4
+
+declare dso_local i32 @f1()
+
+; Do not crash due to store first in BB.
+define dso_local void @f2() {
+for.end:
+  %0 = load i32, i32* @b, align 4
+  ret void
+
+for.body:
+  store i32 %1, i32* @a, align 4
+  %call = call i32 @f1()
+  %cmp = icmp sge i32 %call, 0
+  %1 = load i32, i32* @b, align 4
+  br label %for.body
+}
+
+; Do not crash due to call not before store in BB.
+define dso_local void @f3() {
+for.end:
+  %0 = load i32, i32* @b, align 4
+  ret void
+
+for.body:
+  %t = add i32 %t2, 1
+  store i32 %1, i32* @a, align 4
+  %call = call i32 @f1()
+  %cmp = icmp sge i32 %call, 0
+  %1 = load i32, i32* @b, align 4
+  %t2 = xor i32 %t, 5
+  br label %for.body
+}