[PATCH] Add framework for iterative compilation to llvm

[Hongbin Zheng]

Looks like you need to add some comments to your code :)

================
Comment at: include/llvm/Analysis/DecisionTree.h:44
@@ +43,3 @@
+  DecisionTreeNode *CurrentNode;
+  std::vector<bool> Path;
+  int FunctionFitness;
----------------
You could use llvm::Bitvector instead of std::vector<bool>.

================
Comment at: include/llvm/Analysis/DecisionTree.h:104
@@ +103,3 @@
+public:
+  enum IterativeCompilationPhase {
+    FunctionPhase, ///< Function level optimizations.
----------------
Please briefly explain all these Phases.

================
Comment at: include/llvm/Analysis/DecisionTree.h:158
@@ +157,3 @@
+
+class ModuleDecisionTreeProxies : public ImmutablePass {
+public:
----------------
Maybe you could clarify your code by adding comments like:
"This ImmutablePass provides a persistent place to hold the decision trees (for each functions) during the iterative compilation process ..."

================
Comment at: include/llvm/Analysis/DecisionTree.h:186
@@ +185,3 @@
+  std::map<std::string, DecisionTreeProxy *> DecisionTreeProxies;
+  DecisionTreeProxy *CurrFnDecisionTreeProxy;
+  ModuleDecisionTrees::IterativeCompilationPhase ICPhase;
----------------
Now you have "nameFunction" for the name of the current function and "CurrFnDecisionTreeProxy" for the DecisionTree of the current function. How about add a pointer to Function to  DecisionTreeProxy so that you can always access the current function (as well as its name) when you have a pointer to the current DecisionTree.

Then you can remobe "nameFunction".

You my also need a hook to notify the DecisionTree when the function inlining pass inline and delete a function.

================
Comment at: lib/Analysis/DecisionTree.cpp:37
@@ +36,3 @@
+  FunctionFitness = 0;
+  BestFunctionFitness = 100000;
+  CurrentNode = DecisionTreeRoot;
----------------
Maybe you could use a constant (like MaxDtPriority) here instead of a magic number.

================
Comment at: lib/Analysis/DecisionTree.cpp:90
@@ +89,3 @@
+    CurrentNode->AlternativeDecision = newNode;
+    CurrentNode = newNode;
+    return true;
----------------
Looks like this line of code appears in both branches, you could move it above the "if" statement.

================
Comment at: lib/Analysis/DecisionTree.cpp:103
@@ +102,3 @@
+
+std::vector<bool> DecisionTree::findPath(DecisionTreeNode *node) {
+  std::vector<bool> path;
----------------
In this function you are creating a vector in a function and return it at the end, such an approach need to copy the vector when the function return.

You could ask the caller of this function to pass in an empty vector by reference then fill the vector, such that the signature of this function become:
void DecisionTree::findPath(DecisionTreeNode *node, std::vector<bool> &path) {
...
}


================
Comment at: lib/Analysis/DecisionTree.cpp:254
@@ +253,3 @@
+                               const std::string &data) {
+  std::ofstream outf(fileName.c_str());
+  if (!outf)
----------------
How about use LLVM's streams instead of ofstream?

Maybe you could write your data out in form of JSON, such that you could write python utilities to analyze/debug/render your data. And it is much more human readable.

================
Comment at: lib/Transforms/IPO/Inliner.cpp:456
@@ -443,1 +455,3 @@
+  MDTP = &getAnalysis<ModuleDecisionTreeProxies>();
+  MDTP->setCurrentFunctionAndPhase("", ModuleDecisionTrees::ModulePhase);
   DataLayoutPass *DLP = getAnalysisIfAvailable<DataLayoutPass>();
----------------
This "" looks confusing.

http://reviews.llvm.org/D4723