[PATCH] [MBP] Don't outline short optional branches

[Chandler Carruth]

I'm fine for this to go in behind the flag. It makes the code strictly better.

However, looking at this and thinking about it is really confirming for me that this isn't quite the correct approach. Here are my thoughts, mostly for reference going forward:

Currently, the code is working to select a definite successor to lay out next in the chain. But some of the time, we don't want to do that because there is a non-definite successor that doesn't have a CFG conflict and is small. I feel like it would be better to phrase this whole thing from the perspective of actually *outlining*. Rather than returning a successor early, I wonder if it would work better to *skip* successors which are non-small and optional.

I've tried to think through how this would be implemented, and sadly it doesn't seem straight forward. I really feel like we're struggling with a much more deeply flawed design here and that is why nothing seems to work elegantly.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:388-403
@@ +387,18 @@
+      bool HasShortOptionalBranch = false;
+      for (MachineBasicBlock *Pred : Succ->predecessors()) {
+        // Check whether there is an unplaced optional branch.
+        if (Pred == Succ || (BlockFilter && !BlockFilter->count(Pred)) ||
+            BlockToChain[Pred] == &Chain)
+          continue;
+        // Check whether the optional branch has exactly one BB.
+        if (Pred->pred_size() > 1 || *Pred->pred_begin() != BB)
+          continue;
+        // Check whether the optional branch is small.
+        if (Pred->size() >= OutlineOptionalThreshold)
+          continue;
+
+        HasShortOptionalBranch = true;
+        break;
+      }
+      if (!HasShortOptionalBranch)
+        return Succ;
----------------
I feel like this code would be much easier to read as a lambda predicate.

http://reviews.llvm.org/D8108

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/