[PATCH] Add framework for iterative compilation to llvm

[Tobias Grosser]

I took a brief first look to understand what is happening here.

I am a little surprised that there is not a _single_ comment added in this patch. I believe a couple of comments that document higher level design choices would be very helpful. I looked through the older emails, but some questions are still open:

What exactly forms the decision tree? If we have three points at which decisions are taken, will we have three nodes in the tree or depends the size of the tree on the module/program size? 

What happens if a decision is reached multiple times? E.g. the inliner run on different functions? Can different decisions be taken at each point?

Similarly, is the decision tree statically known or can it change dynamically depending on what kind of decisions have been taken. E.g. if we decide to run dead code elimination, we may not even have code that could be unrolled such that the corresponding unroller decision points may become invalid?

Do you plan to support non-binary decisions. E.g. different unroll factors?

How does all this fit together. Assuming I would like to run this in a JIT compiler, what are the pieces that need to be plugged together to try to iteratively optimize a certain function? 

I also have a couple of stylistic comments:

================
Comment at: include/llvm/Analysis/DecisionTree.h:17
@@ +16,3 @@
+#define LLVM_DECISIONTREE_H
+
+#include <vector>
----------------
Maybe LLVM_ANALYSIS_DECISIONTREE_H?

================
Comment at: lib/Analysis/DecisionTree.cpp:43
@@ +42,3 @@
+  CurrentNode->Priority = 0;
+  CurrentNode->Fitness = 100000;
+  CurrentNode->Depth = 0;
----------------
How has this value be choosen? Does it always need to be in sync with BestFunctionFitness?

================
Comment at: lib/Analysis/DecisionTree.cpp:156
@@ +155,3 @@
+  else {
+    Path = BestPath;
+    return true;
----------------
Write:

} else {

================
Comment at: lib/Analysis/DecisionTree.cpp:206
@@ +205,3 @@
+
+void DecisionTree::applayResults(std::vector<int> &results) {
+  unsigned i;
----------------
Typo: applyResults, no?

================
Comment at: lib/Analysis/DecisionTree.cpp:378
@@ +377,3 @@
+  std::map<std::string, DecisionTree *>::iterator it, e;
+  for (it = DecisionTrees.begin(), e = DecisionTrees.end(); it != e; it++) {
+    DecisionTree *dt = it->second;
----------------
C++11 range loops maybe?

================
Comment at: lib/Target/Mips/MipsFunctionFitness.cpp:46
@@ +45,3 @@
+      getAnalysis<ModuleDecisionTreeProxies>();
+    //DecisionTreeProxy &proxy = proxies.getDecisionTreeProxy(n);
+    proxies.setCurrentFunctionAndPhase(fnName, ModuleDecisionTrees::CodeGenPhase);
----------------
Why is this commented out? (The same for the lines above)

================
Comment at: lib/Target/Mips/MipsFunctionFitness.cpp:72
@@ +71,3 @@
+runOnMachineBasicBlock(MachineBasicBlock &MBB) {
+  unsigned count = 0;
+
----------------
We commonly start variable names with upper-case letters.

http://reviews.llvm.org/D4723