This is an archive of the discontinued LLVM Phabricator instance.

[XRay] Add clang builtin for xray typed events.
ClosedPublic

Authored by kpw on Apr 16 2018, 9:40 PM.

Details

Summary

A clang builtin for xray typed events. Differs from
__xray_customevent(...) by the presence of a type tag that is vended by
compiler-rt in typical usage. This allows xray handlers to expand logged
events with their type description and plugins to process traced events
based on type.

This change depends on D45633 for the intrinsic definition.

Diff Detail

Event Timeline

kpw created this revision.Apr 16 2018, 9:40 PM
kpw updated this revision to Diff 142735.Apr 16 2018, 9:45 PM

Adding a comment to the test to encourage getting the event types from
compiler-rt

dberris requested changes to this revision.Apr 16 2018, 9:48 PM

Thanks, Keith -- we're going to need to add this to the list of instrumentation points we can enable/disable through flags.

lib/CodeGen/CGBuiltin.cpp
3376–3377

Please define a new XRayInstrKind:: for Typed instead of re-using Custom. This allows us to provide finer-grained control on the enabling/disabling of specific flags.

See clang/include/clang/Basic/XRayInstr.h on adding new values to XRayInstrKind and clang/lib/Basic/XRayInstr.cpp in supporting the new value when parsing from -fxray-instrumentation-bundle=. Also, you may need to update XRayArgs.{h,cpp} to teach it to understand how to parse the 'typed' flag value.

This revision now requires changes to proceed.Apr 16 2018, 9:48 PM
kpw added inline comments.Apr 16 2018, 9:49 PM
test/CodeGen/xray-typedevent.cpp
11

FYI: It would be involved to match on more than * for the event type because the actual IR does things like unsigned extension and truncation to i16. The IR looked sane to me though. It's a shame the BuiltIns.def type attributes are defined in terms like short, half, and size_t rather than fixed width types. /shrug

kpw updated this revision to Diff 142737.Apr 16 2018, 10:47 PM

Added flags and bundle options.

kpw added a comment.Apr 16 2018, 11:02 PM

My editor got a bit carried away with automatically clang-formatting lib/CodeGen/CodeGenFunction.cpp. I'll fix that so that I'm not messing up the revision history.

In D45716#1069557, @kpw wrote:

My editor got a bit carried away with automatically clang-formatting lib/CodeGen/CodeGenFunction.cpp. I'll fix that so that I'm not messing up the revision history.

Yes please. :)

Also, if you can add to the tests for instrumentation bundles in test/Driver/XRay/... to ensure that we're properly not seeing the typed events there either, that would be great.

dberris accepted this revision.Apr 16 2018, 11:39 PM
In D45716#1069557, @kpw wrote:

My editor got a bit carried away with automatically clang-formatting lib/CodeGen/CodeGenFunction.cpp. I'll fix that so that I'm not messing up the revision history.

Yes please. :)

Also, if you can add to the tests for instrumentation bundles in test/Driver/XRay/... to ensure that we're properly not seeing the typed events there either, that would be great.

Actually, I see that change too. Ignore that. :)

LGTM (pending the revert of formatting changes in lib/CodeGen/CodeGenFunction.cpp).

This revision is now accepted and ready to land.Apr 16 2018, 11:39 PM
kpw updated this revision to Diff 142790.Apr 17 2018, 9:55 AM

Undoing formatting change.

This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.