[PATCH] D43668: [XRay] [compiler-rt] Implement trampoline and handler for typed xray event tracing.

[Keith via Phabricator via llvm-commits]

kpw added a comment.

These comments were dangling for a while, so I'm posting an update. Currently compiling llvm with a new intrinsic to match. Fingers crossed!



================
Comment at: include/xray/xray_interface.h:88
+extern uint32_t
+__xray_register_event_type(const char *const event_type) noexcept;
+
----------------
dberris wrote:
> kpw wrote:
> > dberris wrote:
> > > I don't think you need the additional `const` for the declaration.
> > > 
> > > Also, I suspect 'noexcept' in the `extern "C" { ... }` section wouldn't make any sense, and would be superfluous.
> > Fair enough about the const.
> > 
> > I wasn't sure if extern "C" only specified linkage. Digging was inconclusive.
> > 
> > https://stackoverflow.com/questions/24362616/does-the-c-standard-mandate-that-c-linkage-functions-are-noexcept
> The noexcept doesn't really add much here, and could be distracting, and is not something we can guarantee either. Is there something in particular you're guarding for with putting the noexcept specifier(s) on these functions?
I've removed them. I'm not sure the stack unwinder even works for functions that use the C calling convention. Since I'm not sure we can support exceptions properly, my thinking was that it might be better to put noexcept so that this behavior just becomes std::terminate if anything is thrown. I don't know what you mean that we can't guarantee no exceptions? We've written the implementations without throwing. These are just handler registration, not handler execution.

Sure there could be a std::bad_alloc, but that's common in noexcept functions and noexcept just means exceptions will be transformed into std::terminate.

I don't want to make the file inconsistent and confusing, by adding the specifier selectively, but it may be more clear to add to all the registration functions since we haven't written implementations that can throw.

I don't think compiler optimizations are very relevant here. They mostly come down to
1. std::move_if_no_except - Not relevant here.
2. Should the compiler generate code for stack unwinding? - Registering is rare compared to invocation anyway.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D43668