[PATCH] Use auto profiler to set branch weights

[Chandler Carruth]

  A few more nit pick comments on the new code.

  I think the big changes left are:

  1) flattening some of the abstractions around profile reading until we have more (different) sources
  2) clarifying that the text-reader is just a line-oriented text file reader
  3) the issue with creating per-instruction sample metadata in addition to branch edge metadata.

  I suspect #1 and #2 are just mechanical and uninteresting. I'm happy to chat w/ you about #3 to get onto the same page if needed.


================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:475-476
@@ +474,4 @@
+  FunctionProfile &FProfile = Profiles[F->getName()];
+  if (FProfile.BlockWeights.find(B) != FProfile.BlockWeights.end())
+    return FProfile.BlockWeights[B];
+
----------------
This does the lookup twice. Store the iterator and use that.

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:485
@@ +484,3 @@
+  }
+  FProfile.BlockWeights[B] = Weight;
+  return Weight;
----------------
Even better, since you're going to compute the weight if it isn't there already, try inserting '0' into the map. If the insert fails, return the weight pointed to by the iterator. If the insert succeeds, update the weight pointed to by the iterator like you do here, and then return that. This does exactly one probe into the hash table.

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:523
@@ +522,3 @@
+
+    llvm::MDBuilder MDB(TI->getContext());
+    llvm::MDNode *WeightsNode = MDB.createBranchWeights(Weights);
----------------
Build this outside of the loop.

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:517
@@ +516,3 @@
+    SmallVector<uint32_t, 4> Weights;
+    for (unsigned I = 0; I < TI->getNumSuccessors(); ++I) {
+      BasicBlock *Succ = TI->getSuccessor(I);
----------------
Cache the number of successors in a loop variable, calling it on every iteration is very expensive.

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:524
@@ +523,3 @@
+    llvm::MDBuilder MDB(TI->getContext());
+    llvm::MDNode *WeightsNode = MDB.createBranchWeights(Weights);
+    TI->setMetadata(llvm::LLVMContext::MD_prof, WeightsNode);
----------------
No need for a variable, just pass it directly to setMetadata.


http://llvm-reviews.chandlerc.com/D2058