[StandardInstrumentations] Handle case where block order changes

[aeubanks]

Summary

Previously we'd go off the end of the BI iterator because we expected
that the relative positions of common blocks before and after were
consistent. That's not always true though, for example with
jump-threading.

[aeubanks]

ping

[jamieschmeiser]

Sorry for the delay, missed this before going on vacation.

llvm/lib/Passes/StandardInstrumentations.cpp

553

I don't understand why you are removing this line. On exiting the loop, (with your change to the loop) BI == BE or *BI == *AI. In the latter case, we still want to advance BI or HandlePotentiallyRemovedData will be called with it on the next time through this loop (AI advances and the above loop will enter when BI != BE and HandlePotentiallyRemovedData will be called with BI before it is advanced). Shouldn't this line be guarded with "if (BI != BE)" instead of being removed?

[aeubanks]

update

[aeubanks]

llvm/lib/Passes/StandardInstrumentations.cpp

553

you're right, fixed

[jamieschmeiser]

LGTM. Thanks.

llvm/lib/Passes/StandardInstrumentations.cpp

if (!BFD.count(*AI)) {
       // This section is new so place it in the queue.  This will cause it
       // to be reported after deleted sections.
       NewDataQueue.emplace_back(&AFD.find(*AI)->getValue());
       ++AI;
       continue;
     }
     // This section is in both; advance and print out any before-only
     // until we get to it.
-    while (*BI != *AI) {
+    // It's possible that this section has moved to be later than before. This
+    // will mess up printing most blocks side by side, but it's a rare case and
+    // it's better than crashing.
+    while (BI != BE && *BI != *AI) {
       HandlePotentiallyRemovedData(*BI);
       ++BI;
     }
     // Report any new sections that were queued up and waiting.
     HandleNewData(NewDataQueue);
 
     const T &AData = AFD.find(*AI)->getValue();
     const T &BData = BFD.find(*AI)->getValue();
     HandlePair(&BData, &AData);
+    if (BI != BE)
       ++BI;
     ++AI;
   }
 
   // Check any remaining before sections to see if they have been removed
   while (BI != BE) {
     HandlePotentiallyRemovedData(*BI);
     ++BI;
   }

llvm/test/Other/ChangePrinters/print-changed-diff-block-ordering-changed.ll

+; RUN: opt -passes=jump-threading %s -disable-output --print-changed=diff 2>&1 | FileCheck %s
+
+; CHECK: IR Dump After JumpThreadingPass
+
+define void @f(i1 %0) {
+  br i1 %0, label %5, label %2
+
+2:                                                ; preds = %1
+  br i1 false, label %b, label %3
+
+3:                                                ; preds = %2
+  %4 = call i64 null()
+  br label %b
+
+b:                                                ; preds = %3, %2
+  br label %5
+
+5:                                                ; preds = %b, %1
+  ret void
+}