This is an archive of the discontinued LLVM Phabricator instance.

[XRay] [compiler-rt] [macosx] Add getTSCFrequency implementation for macOS.
Needs ReviewPublic

Authored by nglevin on Mar 1 2018, 4:29 PM.

Details

Reviewers
dberris
Summary

Use mach_absolute_time and mach_timebase as an alternative to clock_gettime(2) on macOS platforms, to support earlier versions of the OS.

Event Timeline

nglevin created this revision.Mar 1 2018, 4:29 PM
Herald added subscribers: Restricted Project, llvm-commits, delcypher. · View Herald TranscriptMar 1 2018, 4:29 PM
dberris requested changes to this revision.Mar 1 2018, 5:13 PM

Just one suggestion below.

Thanks, @nglevin!

lib/xray/xray_x86_64.cc
103–105

You can turn this into an "assured once" initialisation by doing something like:

static const mach_timebase_info_data_t TI = [] {
  mach_timebase_info_data_t LocalTI;
  mach_timebase_info(&LocalTI);
  return LocalTI;
}();

This way you let the compiler do the ensuring that the object is initialised once, and only on the first time it's invoked, in a thread-safe manner.

This revision now requires changes to proceed.Mar 1 2018, 5:13 PM
kubamracek added inline comments.
lib/xray/xray_x86_64.cc
109

Why are we returning "current time" when the function is supposed to return "TSC frequency"? How is a current timestamp a frequency value?

nglevin updated this revision to Diff 136654.Mar 1 2018, 5:24 PM

Transform mach_timebase_info_data_t into an "assured once" initialisation, per feedback.

nglevin updated this revision to Diff 136663.Mar 1 2018, 6:27 PM
nglevin marked an inline comment as done.

Add Mac-specific fallback logic to readTSC, retrieve "getTSCFrequency" via sysctlbyname using the XNU specific name of "machdep.tsc.frequency".

nglevin updated this revision to Diff 136664.Mar 1 2018, 6:28 PM

Correct Mac sysctl name.

nglevin marked an inline comment as done.Mar 1 2018, 6:46 PM
nglevin added inline comments.
lib/xray/xray_x86_64.cc
109

That was supposed to be the fallback logic that I had discussed earlier with Dean.

My fault for putting this in the wrong file, where here, I should be using sysctl directly instead of a clock_gettime()-like workaround. As was previously done in the *BSD code path.

Corrected.

dberris added inline comments.Mar 4 2018, 4:36 PM
lib/xray/xray_x86_64.cc
85

Please follow the naming convention around this file -- local variables are TitleCased.

Also, use a function-local static constexpr character array instead of a non-const pointer.

static constexpr char SysCtlTSCName[] = "...";