[PATCH] Tolerate unmangled names in sample profiles.

Chandler Carruth chandlerc at gmail.com
Mon Mar 17 10:50:55 PDT 2014



================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:497-509
@@ +496,15 @@
+    //
+    // FIXME: this does not detect all cases. It only works with unmangled
+    // names that contain "invalid" characters (spaces, ':', '<', etc).
+    if (FName.substr(0, 2) != "_Z") {
+      // Only check names that do not start with _Z. Anything starting
+      // with _Z should be a properly mangled name.
+      Regex InvalidChars("[:,<>-]");
+      if (InvalidChars.match(FName))
+        reportParseWarning(LineIt.line_number(),
+                           "Function '" + FName +
+                               "' is not properly mangled. No "
+                               "samples will be collected "
+                               "for it");
+    }
+
----------------
Especially based on the FIXME I wonder whether its even worth warning about this.

Essentially, I'd like to either warn about every function mentioned in the profile and not consumed here (including ones containing invalid characters for function names), or none of them. Would that policy make sense? Is there a particular reason to emit warnings?

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:499-501
@@ +498,5 @@
+    // names that contain "invalid" characters (spaces, ':', '<', etc).
+    if (FName.substr(0, 2) != "_Z") {
+      // Only check names that do not start with _Z. Anything starting
+      // with _Z should be a properly mangled name.
+      Regex InvalidChars("[:,<>-]");
----------------
Not in a freestanding implementation? Not on Windows?

Unsure why this is a valid assumption. See my comment below, but I wonder if it make sense to just always skip function names with characters we can't use in the primary parse...

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:502
@@ +501,3 @@
+      // with _Z should be a properly mangled name.
+      Regex InvalidChars("[:,<>-]");
+      if (InvalidChars.match(FName))
----------------
Please factor regex parsing out of the loop.


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

BRANCH
  accept-unmangled

ARCANIST PROJECT
  llvm



More information about the llvm-commits mailing list