From 029ab41a28706e5cb56dc5ade329046b43009bf3 Mon Sep 17 00:00:00 2001
From: Mikael Holmen <mikael.holmen@ericsson.com>
Date: Tue, 27 Jun 2017 05:32:13 +0000
Subject: [PATCH] [Reassociate] Make sure EraseInst sets MadeChange

Summary:
EraseInst didn't report that it made IR changes through MadeChange.

It is essential that changes to the IR are reported correctly,
since for example ReassociatePass::run() will indicate that all
analyses are preserved otherwise.
And the CGPassManager determines if the CallGraph is up-to-date
based on status from InstructionCombiningPass::runOnFunction().

Reviewers: craig.topper, rnk, davide

Reviewed By: rnk, davide

Subscribers: llvm-commits

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@306368 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Transforms/Scalar/Reassociate.cpp         |  2 ++
 .../Reassociate/erase_inst_made_change.ll     | 29 +++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 test/Transforms/Reassociate/erase_inst_made_change.ll

diff --git a/lib/Transforms/Scalar/Reassociate.cpp b/lib/Transforms/Scalar/Reassociate.cpp
index 6da551bd7ef..cdba0062953 100644
--- a/lib/Transforms/Scalar/Reassociate.cpp
+++ b/lib/Transforms/Scalar/Reassociate.cpp
@@ -1894,6 +1894,8 @@ void ReassociatePass::EraseInst(Instruction *I) {
         Op = Op->user_back();
       RedoInsts.insert(Op);
     }
+
+  MadeChange = true;
 }
 
 // Canonicalize expressions of the following form:
diff --git a/test/Transforms/Reassociate/erase_inst_made_change.ll b/test/Transforms/Reassociate/erase_inst_made_change.ll
new file mode 100644
index 00000000000..febb9447e2b
--- /dev/null
+++ b/test/Transforms/Reassociate/erase_inst_made_change.ll
@@ -0,0 +1,29 @@
+; RUN: opt < %s -inline -reassociate -S | FileCheck %s
+
+; This test case exposed a bug in reassociate where EraseInst's
+; removal of a dead call wasn't recognized as changing the IR.
+; So when runOnFunction propagated the "made changes" upwards
+; to the CallGraphSCCPass it signalled that no changes had been
+; made, so CallGraphSCCPass assumed that the old CallGraph,
+; as known by that pass manager, still was up-to-date.
+;
+; This was detected as an assert when trying to remove the
+; no longer used function 'bar' (due to incorrect reference
+; count in the CallGraph).
+
+define void @foo() {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret void
+entry:
+  call void @bar()
+  ret void
+}
+
+define internal void @bar() noinline nounwind readnone {
+; CHECK-NOT: bar
+entry:
+  ret void
+}
+
+
-- 
2.50.1