[PATCH] Use auto profiler to set branch weights

Benjamin Kramer benny.kra at gmail.com
Tue Nov 12 04:44:38 PST 2013


  Comments from the peanut gallery.


================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:314
@@ +313,3 @@
+    Loader.reportParseError("Expected a number, found " + Line);
+  int NumSymbols = atoi(Line.c_str());
+  for (int I = 0; I < NumSymbols; I++) {
----------------
I think we prefer StringRef's getAsInteger to atoi. It also does error checking so you don't have to do it with a regex. That would also eliminate the std::string conversions above.

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:336
@@ +335,3 @@
+    StringRef FName = Matches[1];
+    unsigned NumSamples = atoi(Matches[2].str().c_str());
+    unsigned NumHeadSamples = atoi(Matches[3].str().c_str());
----------------
More atoi()s.

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:377
@@ +376,3 @@
+  unsigned LOffset = Inst.getDebugLoc().getLine() - FirstLineno + 1;
+  return (BodySamples.find(LOffset) != BodySamples.end()) ? BodySamples[LOffset]
+                                                          : 0;
----------------
This is equivalent to BodySamples.lookup(LOffset)

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:435
@@ +434,3 @@
+  unsigned FirstLineno = FirstInst.getDebugLoc().getLine();
+  llvm::MDBuilder MDB(F.getContext());
+
----------------
Why the extra llvm:: qualification? We're in a using namespace llvm environment anyways.


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



More information about the llvm-commits mailing list