[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