[llvm] r305725 - [InstCombine] Make sure AddReachableCodeToWorklist sets MadeIRChange

Bjorn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 19 11:00:27 PDT 2017


Author: bjope
Date: Mon Jun 19 13:00:27 2017
New Revision: 305725

URL: http://llvm.org/viewvc/llvm-project?rev=305725&view=rev
Log:
[InstCombine] Make sure AddReachableCodeToWorklist sets MadeIRChange

Summary:
Some optimizations in AddReachableCodeToWorklist did not update
the MadeIRChange state. This could happen both when removing
trivially dead instructions (DCE) and at constant folds.

It is essential that changes to the IR is reported correctly,
since for example InstCombinePass::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().

The new test case early_dce_clobbers_callgraph.ll is a reproducer
for some asserts that started to trigger after changes in the
inliner in r305245. With this patch the test case passes again.

Reviewers: sanjoy, craig.topper, dblaikie

Reviewed By: craig.topper

Subscribers: llvm-commits

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

Added:
    llvm/trunk/test/Transforms/InstCombine/early_constfold_changes_IR.ll
    llvm/trunk/test/Transforms/InstCombine/early_dce_clobbers_callgraph.ll
Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp

Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=305725&r1=305724&r2=305725&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Mon Jun 19 13:00:27 2017
@@ -3012,6 +3012,7 @@ static bool AddReachableCodeToWorklist(B
         ++NumDeadInst;
         DEBUG(dbgs() << "IC: DCE: " << *Inst << '\n');
         Inst->eraseFromParent();
+        MadeIRChange = true;
         continue;
       }
 
@@ -3025,6 +3026,7 @@ static bool AddReachableCodeToWorklist(B
           ++NumConstProp;
           if (isInstructionTriviallyDead(Inst, TLI))
             Inst->eraseFromParent();
+          MadeIRChange = true;
           continue;
         }
 

Added: llvm/trunk/test/Transforms/InstCombine/early_constfold_changes_IR.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/early_constfold_changes_IR.ll?rev=305725&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/early_constfold_changes_IR.ll (added)
+++ llvm/trunk/test/Transforms/InstCombine/early_constfold_changes_IR.ll Mon Jun 19 13:00:27 2017
@@ -0,0 +1,20 @@
+; This run line verifies that we get the expected constant fold.
+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+; This run line verifies that InstructionCombiningPass::runOnFunction reports
+; this as a modification of the IR.
+; RUN: opt < %s -instcombine -disable-output -debug-pass=Details 2>&1 | FileCheck %s --check-prefix=DETAILS
+
+define i32 @foo(i32 %arg) #0 {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[AND:%.*]] = and i32 [[ARG:%.*]], 7
+; CHECK-NEXT:    ret i32 [[AND]]
+;
+entry:
+  %or = or i32 0, 7
+  %and = and i32 %arg, %or
+  ret i32 %and
+}
+
+; DETAILS:  Made Modification 'Combine redundant instructions' on Function 'foo'

Added: llvm/trunk/test/Transforms/InstCombine/early_dce_clobbers_callgraph.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/early_dce_clobbers_callgraph.ll?rev=305725&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/early_dce_clobbers_callgraph.ll (added)
+++ llvm/trunk/test/Transforms/InstCombine/early_dce_clobbers_callgraph.ll Mon Jun 19 13:00:27 2017
@@ -0,0 +1,31 @@
+; RUN: opt < %s -inline -instcombine -S | FileCheck %s
+
+; This test case exposed a bug in instcombine where the early
+; DCE of a 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).
+
+attributes #0 = { noinline norecurse nounwind readnone }
+
+define void @foo() #0 {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret void
+;
+entry:
+  %call = call i32 @bar()
+  ret void
+}
+
+define internal i32 @bar() #0 {
+; CHECK-NOT: bar
+entry:
+  ret i32 42
+}
+




More information about the llvm-commits mailing list