[PATCH] Add control dependence computation that computes control dependence equivalence classes.

[Daniel Berlin]

================
Comment at: include/llvm/Analysis/ControlDependence.h:59-64
@@ +58,8 @@
+
+  /// \brief Return the control dependence class number for \p BB.  All basic
+  /// blocks with the same class number have the same control dependences
+  unsigned getClassNumber(BasicBlock *BB) {
+    assert(Computed && "Trying to get equivalence classes before computation");
+    return BlockInfo[BB].ClassNumber;
+  }
+
----------------
chandlerc wrote:
> Is 'unsigned' enough? (I can believe it is, I just want to surface the question.)
It should be.
We will never have more classes than basic blocks, and we already assume O(BasicBlocks) fits in unsigned elsewhere in the compiler. 

================
Comment at: lib/Analysis/ControlDependence.cpp:21-24
@@ +20,6 @@
+using namespace llvm;
+char ControlDependence::ID = 0;
+#define DEBUG_TYPE "controlequiv"
+INITIALIZE_PASS(ControlDependence, "controldep",
+                "Control Dependence Construction", true, true);
+
----------------
chandlerc wrote:
> Why are these jammed in here with headers and usings?
Fixed.

================
Comment at: lib/Analysis/ControlDependence.cpp:29
@@ +28,3 @@
+// numbers for all participating blocks. Takes O(E) time and O(N) space.
+
+bool ControlDependence::runOnFunction(Function &F) {
----------------
chandlerc wrote:
> Weird whitspace...
Fixed. 

================
Comment at: lib/Analysis/ControlDependence.cpp:122-134
@@ +121,15 @@
+        // and preorder-visit
+        if (PredData.OnStack) {
+          DEBUG(dbgs() << "Maybe visit pred backedge\n");
+          if (Pred != Entry.ParentBlock) {
+            BlockInfo[B].Backedges.push_back(Pred);
+            visitBackedge(B, Pred, PredDirection);
+          }
+
+        } else {
+          BlockInfo[B].Children.push_back(Pred);
+          pushDFS(Stack, Pred, B, PredDirection);
+          visitPre(Pred);
+        }
+        continue;
+      }
----------------
chandlerc wrote:
> This looks like it could be cleaned up with early-continue patterns.
I'll take a looksee and see what can be done

================
Comment at: lib/Analysis/ControlDependence.cpp:200-201
@@ +199,4 @@
+      combined_succ_end(B, BlockResultData.FakeSuccEdges), From, B});
+}
+/// \brief Pop an entry from the DFS stack.
+void ControlDependence::popDFS(DFSStack &Stack, const BasicBlock *B) {
----------------
chandlerc wrote:
> Missing vertical whitespace.
fixed

================
Comment at: lib/Analysis/ControlDependence.cpp:260-261
@@ +259,4 @@
+      BII = Info.BracketIterators.erase(BII);
+    } else
+      ++BII;
+  }
----------------
chandlerc wrote:
> Keep braces symmetric.
fixed

================
Comment at: lib/Analysis/ControlDependence.cpp:311-321
@@ +310,13 @@
+    else {
+      // Get the min and the max through our children
+      auto hi1iter = std::min_element(
+          Info.Children.begin(), Info.Children.end(),
+          [this](const BasicBlock *A, const BasicBlock *B) {
+            return BlockInfo[A].DFSNumber < BlockInfo[B].DFSNumber;
+          });
+      auto hi2iter = std::max_element(
+          Info.Children.begin(), Info.Children.end(),
+          [this](const BasicBlock *A, const BasicBlock *B) {
+            return BlockInfo[A].DFSNumber < BlockInfo[B].DFSNumber;
+          });
+      Hi1 = *hi1iter;
----------------
chandlerc wrote:
> Variable naming: Hi1Iter
Fixed these

================
Comment at: lib/Analysis/ControlDependence.cpp:376-377
@@ +375,4 @@
+  DEBUG(dbgs() << "Assigned class number is " << Info.ClassNumber << "\n");
+}
+void ControlDependence::visitPost(const BasicBlock *B,
+                                  const BasicBlock *ParentBlock,
----------------
chandlerc wrote:
> Again, missing whitespace.
Fixed

================
Comment at: lib/Analysis/ControlDependence.cpp:390-402
@@ +389,15 @@
+
+  // By the time we hit this node, we are guaranteed these iterators will point
+  // into our list, because it must have been spliced into us.
+  for (auto BII = Info.BracketIterators.begin(),
+            BIE = Info.BracketIterators.end();
+       BII != BIE;) {
+    BracketList::iterator &BLI = *BII;
+    Bracket &BR = *BLI;
+    if (BR.To == B && BR.Direction != Direction) {
+      Info.BList.erase(BLI);
+      BII = Info.BracketIterators.erase(BII);
+    } else
+      ++BII;
+  }
+
----------------
chandlerc wrote:
> This code seems completely duplicated with the above?
Yes, moved to a helper function

================
Comment at: lib/Analysis/ControlDependence.cpp:410
@@ +409,3 @@
+    BracketList &ParentBList = BlockInfo[ParentBlock].BList;
+    // TODO
+    ParentBList.splice(ParentBList.end(), BList);
----------------
chandlerc wrote:
> TODO what?
Fixed
(incomplete deletion, it used to be "make this O(1)")


================
Comment at: lib/Analysis/ControlDependence.cpp:433-434
@@ +432,4 @@
+  BlockInfo[To].BracketIterators.push_back(Place);
+}
+FunctionPass *llvm::createControlDependencePass() {
+  return new ControlDependence();
----------------
chandlerc wrote:
> Missing whitespace.
Fixed

http://reviews.llvm.org/D8568

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