dberris added inline comments.
================
Comment at: clang/include/clang/Frontend/CodeGenOptions.h:110
+ enum XRayInstrumentationPointBundle {
+ XRay_All, // Always emit all the instrumentation points.
----------------
pelikan wrote:
> To me, this feels like a bitfield would be a better match.
> All = Function | Custom
> None = 0
Thought about that, but the ergonomics from the user-side isn't as good. Having
to know about which kinds of sleds specifically to enable seems much harder to
explain. Using bundles that we control from the beginning keeps this much
simpler.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3248
+ // custom events.
+ {
+ auto XRayBundle = CGM.getCodeGenOpts().getXRayInstrumentationBundle();
----------------
echristo wrote:
> I'd probably spell this code like the block above it rather than this.
The codegen options don't work like pointers, and we can't use C++17 if
initialisers yet. Would have loved this to be:
```
if (auto XRayBundle = ...; XRayBundle == ... || XRayBundle == ...)
return RValue::getIgnored();
```
Removed the scoping instead.
================
Comment at: clang/lib/Driver/XRayArgs.cpp:62
+ << (std::string(XRayInstrumentOption) +
+ " on non-supported target OS");
}
----------------
pelikan wrote:
> 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).
Probably not today. Good idea though, could be a different patch.
https://reviews.llvm.org/D44970
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
