pelikan added a comment.
I would probably add more tests for the different configurations, but I suspect
more diffs are coming after this.
================
Comment at: clang/include/clang/Driver/Options.td:1112
+ Group<f_Group>, Flags<[CC1Option]>,
+ HelpText<"Select which bundle of XRay instrumentation points to emit.
Options: all, none, function-extents, custom-only.">;
+
----------------
I'd suggest making them "fn-only" and "custom-only", or just plain "function"
and "custom".
================
Comment at: clang/include/clang/Driver/Options.td:1112
+ Group<f_Group>, Flags<[CC1Option]>,
+ HelpText<"Select which bundle of XRay instrumentation points to emit.
Options: all, none, function-extents, custom-only.">;
+
----------------
pelikan wrote:
> I'd suggest making them "fn-only" and "custom-only", or just plain "function"
> and "custom".
I also don't get why "none" is an option here, if it's equivalent to
-fnoxray-instrument. Why duplicate the functionality and add more points the
users will have to debug?
================
Comment at: clang/include/clang/Frontend/CodeGenOptions.h:110
+ enum XRayInstrumentationPointBundle {
+ XRay_All, // Always emit all the instrumentation points.
----------------
To me, this feels like a bitfield would be a better match.
All = Function | Custom
None = 0
================
Comment at: clang/lib/Driver/XRayArgs.cpp:62
+ << (std::string(XRayInstrumentOption) +
+ " on non-supported target OS");
}
----------------
I would also print the triple. Something like:
"-fomit-bugs is not supported on target win64-ppc-none"
will be much more informative, especially when you collect logs from build
machines on lots of architectures (like Linux distro/BSD package builders do).
================
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" &&
----------------
echristo wrote:
> How about a more descriptive name here and a string switch below?
+1
https://reviews.llvm.org/D44970
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
