Check the loop header before updating it in UpdateAnalysisInformation

[anna]

Summary

In UpdateAnalysisInformation, when passed in predecessors and the Old Basic
Block and the NewBasicBlock (that is split from the old one), we try to identify
if the loopheader has changed. The loop header changes to newBB if the OldBB was
actually the loop header.
This patch checks for that before changing the loop header. Specifically, we saw
a failure in IRCE where the loop header was changed incorrectly through a call
to SplitLandingPadPredecessors.

[anna]

One thing to mention: I think currently there's no case where SplitMakesNewLoopHeader is true when the OldBB is not a header in UpdateAnalysisInformation. This happens only in IRCE. However, it is possible at some point, when a subloop does not have dedicated exits, and we incorrectly update the header of the subloop.

[anna]

We're looking at fixing this in IRCE itself.

lib/Transforms/Utils/BasicBlockUtils.cpp

for (BasicBlock *Pred : Preds) {
           InnermostPredLoop = PredLoop;
       }
     }
 
     if (InnermostPredLoop)
       InnermostPredLoop->addBasicBlockToLoop(NewBB, *LI);
   } else {
     L->addBasicBlockToLoop(NewBB, *LI);
-    if (SplitMakesNewLoopHeader)
+    // At this point, we know that some of the `preds` of OldBB are from within
+    // the loop L (since IsLoopEntry is false) and there's atleast one pred from
+    // outside the loop (SplitMakesNewLoopHeader is true). This does not mean
+    // that OldBB is the header of L by default (it could be an exit block of L
+    // where L is a subloop that does not satisfy loop-simplify form). We need
+    // to check OldBB is the header before changing the header to be the block
+    // split from OldBB (i.e. NewBB).
+    // FIXME: Should the header check be an assert?
+    if (SplitMakesNewLoopHeader && L->getHeader() == OldBB)
       L->moveToHeader(NewBB);
   }
 }
 
 /// Update the PHI nodes in OrigBB to include the values coming from NewBB.
 /// This also updates AliasAnalysis, if available.
 static void UpdatePHINodes(BasicBlock *OrigBB, BasicBlock *NewBB,
                            ArrayRef<BasicBlock *> Preds, BranchInst *BI,

test/Transforms/IRCE/correct-loop-info.ll

+; RUN: opt -irce < %s 
+
+; REQUIRES: asserts
+
+; IRCE creates the pre and post loop, and invokes the
+; SplitLandingPadPredecessor. Make sure that the update to the loopinfo does not
+; incorrectly move the split block as a header (since the block it was split from
+; was not the loop header).
+source_filename = "correct-loop-info.ll"
+
+define void @baz() personality i32* ()* @ham {
+bb:
+  br label %outerheader
+
+outerheader:                                              ; preds = %bb14, %bb
+  %tmp = icmp slt i32 undef, 84
+  br i1 %tmp, label %bb2, label %bb16
+
+bb2:                                              ; preds = %outerheader
+  br label %innerheader
+
+innerheader:                                              ; preds = %bb8, %bb2
+  %tmp4 = phi i32 [ %tmp6, %bb8 ], [ undef, %bb2 ]
+  invoke void @pluto() 
+          to label %bb5 unwind label %outer_exiting
+
+bb5:                                              ; preds = %innerheader
+  %tmp6 = add i32 %tmp4, 1
+  %tmp7 = icmp ult i32 %tmp6, 0
+  br i1 %tmp7, label %bb8, label %exit3
+
+bb8:                                              ; preds = %bb5
+  %tmp9 = icmp slt i32 %tmp6, 84
+  br i1 %tmp9, label %innerheader, label %bb13
+
+outer_exiting:                                             ; preds = %innerheader
+  %tmp11 = landingpad { i8*, i32 }
+          cleanup
+  switch i32 undef, label %exit2 [
+    i32 142, label %bb14
+    i32 448, label %exit
+  ]
+
+exit3:                                             ; preds = %bb5
+  ret void
+
+bb13:                                             ; preds = %bb8
+  unreachable
+
+bb14:                                             ; preds = %outer_exiting
+  br label %outerheader
+
+exit:                                             ; preds = %outer_exiting
+  ret void
+
+bb16:                                             ; preds = %outerheader
+  ret void
+
+exit2:                                             ; preds = %outer_exiting
+  ret void
+}
+
+declare i32* @ham()
+
+declare void @pluto()