[PATCH] D39114: [XRay][darwin] Initial XRay in Darwin Support

Dean Michael Berris via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 28 01:54:54 PST 2017


dberris added a comment.

In https://reviews.llvm.org/D39114#930461, @kubamracek wrote:

> Hi, can you split the patch and send individual changes as separate patches? It's getting hard to follow. And I think we should land the parts that are "safe to land", e.g. the ASM changes or some of the NFC whitespace changes. The Darwin-specific changes in tests can also be landed (but don't enable the tests on Darwin, yet).


Yes, I'm going to be breaking this up into smaller parts -- just trying to see where it's going at this point.

I've just finally gotten to the point where I can get back to this again. :)

In https://reviews.llvm.org/D39114#930477, @kubamracek wrote:

> And we should probably introduce files named `xray_linux.cc` and `xray_mac.cc` to contain platform-specific functions.


I fully agree. That should happen in smaller steps.

Expect smaller reviews coming soon, from breaking this patch up. :D



================
Comment at: compiler-rt/lib/xray/xray_x86_64.cc:25
+  size_t Len = 0;
+  if (sysctlbyname("hw.cpufrequency_max", &CPUFreq, &Len, NULL, 0) == -1) {
+    Report("Unable to determine CPU frequency for TSC accounting; errno = %d\n",
----------------
krytarowski wrote:
> Is it OK for you to call here internally malloc(3)? At least the NetBSD version calls it.
Good point. We should be caching this information earlier on in the process (i.e. before we start tracing) to avoid deadlocks in case we have an instrumented malloc implementation. Let me put a `TODO` or at least make sure we're calling this at a point that is safe (probably at initialisation time).


https://reviews.llvm.org/D39114





More information about the llvm-commits mailing list