[PATCH] D34616: [Reassociate] Make sure EraseInst sets MadeChange

Mikael Holmén via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 26 06:07:12 PDT 2017


uabelho created this revision.

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

It is essential that changes to the IR is 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().


https://reviews.llvm.org/D34616

Files:
  lib/Transforms/Scalar/Reassociate.cpp
  test/Transforms/Reassociate/erase_inst_made_change.ll


Index: test/Transforms/Reassociate/erase_inst_made_change.ll
===================================================================
--- /dev/null
+++ 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 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
+}
+
+
Index: lib/Transforms/Scalar/Reassociate.cpp
===================================================================
--- lib/Transforms/Scalar/Reassociate.cpp
+++ lib/Transforms/Scalar/Reassociate.cpp
@@ -1894,6 +1894,8 @@
         Op = Op->user_back();
       RedoInsts.insert(Op);
     }
+
+  MadeChange = true;
 }
 
 // Canonicalize expressions of the following form:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D34616.103933.patch
Type: text/x-patch
Size: 1410 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170626/6b47ad90/attachment.bin>


More information about the llvm-commits mailing list