[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

Eric Christopher via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 28 09:50:39 PDT 2018


echristo added inline comments.


================
Comment at: clang/include/clang/Frontend/CodeGenOptions.h:113
+    XRay_None,            // Emit none of the instrumentation points.
+    XRay_FunctionExtents, // Only emit function entry/exit instrumentation
+                          // points.
----------------
"function" might spell easier? :)


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3248
+    // custom events.
+    {
+      auto XRayBundle = CGM.getCodeGenOpts().getXRayInstrumentationBundle();
----------------
I'd probably spell this code like the block above it rather than this.


================
Comment at: clang/lib/Driver/XRayArgs.cpp:61
       D.Diag(diag::err_drv_clang_unsupported)
-          << (std::string(XRayInstrumentOption) + " on non-supported target OS");
+          << (std::string(XRayInstrumentOption) +
+              " on non-supported target OS");
----------------
Extraneous reformat.


================
Comment at: clang/lib/Driver/XRayArgs.cpp:86
+            Args.getLastArg(options::OPT_fxray_instrumentation_bundle)) {
+      StringRef B = A->getValue();
+      if (B != "all" && B != "none" && B != "function-extents" &&
----------------
How about a more descriptive name here and a string switch below?


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:452
+  StringRef V = A->getValue();
+  if (V == "all")
+    return CodeGenOptions::XRayInstrumentationPointBundle::XRay_All;
----------------
StringSwitch maybe?


https://reviews.llvm.org/D44970





More information about the cfe-commits mailing list