[SimplifyCFG] Fix crash in the EXPENSIVE_CHECKS build

[jroelofs]

Summary

SimplifyCFG was incorrectly reporting to the pass manager that it had not made changes after folding away a PHI. This is detected in the EXPENSIVE_CHECKS build when the function's hash changes.

[lebedev.ri]

LG, thank you!

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
 
   // Loop over the PHI's seeing if we can promote them all to select
   // instructions.  While we are at it, keep track of the instructions
   // that need to be moved to the dominating block.
   SmallPtrSet<Instruction *, 4> AggressiveInsts;
   int BudgetRemaining =
       TwoEntryPHINodeFoldingThreshold * TargetTransformInfo::TCC_Basic;
 
+  bool Changed = false;
   for (BasicBlock::iterator II = BB->begin(); isa<PHINode>(II);) {
     PHINode *PN = cast<PHINode>(II++);
     if (Value *V = SimplifyInstruction(PN, {DL, PN})) {
       PN->replaceAllUsesWith(V);
       PN->eraseFromParent();
+      Changed = true;
       continue;
     }
 
     if (!DominatesMergePoint(PN->getIncomingValue(0), BB, AggressiveInsts,
                              BudgetRemaining, TTI) ||
         !DominatesMergePoint(PN->getIncomingValue(1), BB, AggressiveInsts,
                              BudgetRemaining, TTI))
-      return false;
+      return Changed;
   }
 
   // If we folded the first phi, PN dangles at this point.  Refresh it.  If
   // we ran out of PHIs then we simplified them all.
   PN = dyn_cast<PHINode>(BB->begin());
   if (!PN)
     return true;
 
static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
   // of the incoming values is an 'not' and another one is freely invertible.
   // These can often be turned into switches and other things.
   if (PN->getType()->isIntegerTy(1) &&
       (isa<BinaryOperator>(PN->getIncomingValue(0)) ||
        isa<BinaryOperator>(PN->getIncomingValue(1)) ||
        isa<BinaryOperator>(IfCond)) &&
       !CanHoistNotFromBothValues(PN->getIncomingValue(0),
                                  PN->getIncomingValue(1)))
-    return false;
+    return Changed;
 
   // If all PHI nodes are promotable, check to make sure that all instructions
   // in the predecessor blocks can be promoted as well. If not, we won't be able
   // to get rid of the control flow, so it's not worth promoting to select
   // instructions.
   BasicBlock *DomBlock = nullptr;
   BasicBlock *IfBlock1 = PN->getIncomingBlock(0);
   BasicBlock *IfBlock2 = PN->getIncomingBlock(1);
   if (cast<BranchInst>(IfBlock1->getTerminator())->isConditional()) {
     IfBlock1 = nullptr;
   } else {
     DomBlock = *pred_begin(IfBlock1);
     for (BasicBlock::iterator I = IfBlock1->begin(); !I->isTerminator(); ++I)
       if (!AggressiveInsts.count(&*I) && !isa<DbgInfoIntrinsic>(I)) {
         // This is not an aggressive instruction that we can promote.
         // Because of this, we won't be able to get rid of the control flow, so
         // the xform is not worth it.
-        return false;
+        return Changed;
       }
   }
 
   if (cast<BranchInst>(IfBlock2->getTerminator())->isConditional()) {
     IfBlock2 = nullptr;
   } else {
     DomBlock = *pred_begin(IfBlock2);
     for (BasicBlock::iterator I = IfBlock2->begin(); !I->isTerminator(); ++I)
       if (!AggressiveInsts.count(&*I) && !isa<DbgInfoIntrinsic>(I)) {
         // This is not an aggressive instruction that we can promote.
         // Because of this, we won't be able to get rid of the control flow, so
         // the xform is not worth it.
-        return false;
+        return Changed;
       }
   }
   assert(DomBlock && "Failed to find root DomBlock");
 
   LLVM_DEBUG(dbgs() << "FOUND IF CONDITION!  " << *IfCond
                     << "  T: " << IfTrue->getName()
                     << "  F: " << IfFalse->getName() << "\n");
 

llvm/test/Transforms/SimplifyCFG/two-entry-phi-fold-crash.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -S -simplifycfg -debug-pass=Details 2>&1 | FileCheck %s
+
+;; Check the report from the pass manager, telling us whether simplifycfg
+;; reported the "changed" status correctly:
+; CHECK: Made Modification 'Simplify the CFG' on Function 'wibble'...
+
+declare i32 @blam(i8*, i32)
+
+define i32 @wibble(i8* %arg, i8** %arg1) {
+; CHECK-LABEL: @wibble(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    br label [[BB2:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    [[BORG:%.*]] = phi i32 [ 0, [[BB:%.*]] ], [ [[BORG]], [[BB8:%.*]] ]
+; CHECK-NEXT:    [[BORG3:%.*]] = phi i32 [ 8, [[BB]] ], [ [[BORG10:%.*]], [[BB8]] ]
+; CHECK-NEXT:    [[BORG4:%.*]] = tail call i32 @blam(i8* [[ARG:%.*]], i32 [[BORG]])
+; CHECK-NEXT:    [[BORG5:%.*]] = icmp eq i32 [[BORG4]], 0
+; CHECK-NEXT:    br i1 [[BORG5]], label [[BB8]], label [[BB6:%.*]]
+; CHECK:       bb6:
+; CHECK-NEXT:    [[BORG7:%.*]] = load i8*, i8** [[ARG1:%.*]], align 4
+; CHECK-NEXT:    br label [[BB8]]
+; CHECK:       bb8:
+; CHECK-NEXT:    [[BORG10]] = phi i32 [ [[BORG4]], [[BB6]] ], [ [[BORG3]], [[BB2]] ]
+; CHECK-NEXT:    [[BORG11:%.*]] = icmp ult i32 [[BORG]], 2
+; CHECK-NEXT:    br i1 [[BORG11]], label [[BB2]], label [[BB12:%.*]]
+; CHECK:       bb12:
+; CHECK-NEXT:    ret i32 1
+;
+bb:
+  br label %bb2
+
+bb2:                                              ; preds = %bb8, %bb
+  %borg = phi i32 [ 0, %bb ], [ %borg9, %bb8 ]
+  %borg3 = phi i32 [ 8, %bb ], [ %borg10, %bb8 ]
+  %borg4 = tail call i32 @blam(i8* %arg, i32 %borg)
+  %borg5 = icmp eq i32 %borg4, 0
+  br i1 %borg5, label %bb8, label %bb6
+
+bb6:                                              ; preds = %bb2
+  %borg7 = load i8*, i8** %arg1, align 4
+  br label %bb8
+
+bb8:                                              ; preds = %bb6, %bb2
+  %borg9 = phi i32 [ %borg, %bb6 ], [ %borg, %bb2 ]
+  %borg10 = phi i32 [ %borg4, %bb6 ], [ %borg3, %bb2 ]
+  %borg11 = icmp ult i32 %borg9, 2
+  br i1 %borg11, label %bb2, label %bb12
+
+bb12:                                             ; preds = %bb8
+  ret i32 1
+}