[PATCH] D52162: [XRay] Support for Fuchsia

[Roland McGrath via Phabricator via llvm-commits]

mcgrathr added inline comments.


================
Comment at: compiler-rt/lib/xray/xray_allocator.h:56
+  if (Status != ZX_OK) {
+    _zx_handle_close(Vmo);
+    if (Verbosity())
----------------
This needs to be outside (before) the if.


================
Comment at: compiler-rt/lib/xray/xray_allocator.h:106
+  if (Status != ZX_OK) {
+    _zx_handle_close(Vmo);
+    if (Verbosity())
----------------
ditto


================
Comment at: compiler-rt/lib/xray/xray_utils.cc:40
+#if SANITIZER_FUCHSIA
+static const char ProfileSinkName[] = "llvm-xray";
+
----------------
phosek wrote:
> mcgrathr wrote:
> > I'd use `inline constexpr const char* ProfileSinkName = "llvm-xray";`.
> > That makes the string constant mergeable.
> compiler-rt uses C++11 but `inline constexpr` is a C++17 construct.
Since this is not inside a class, I don't think inline actually makes a difference.  The advice to use a string literal rather than a const array stands.


================
Comment at: compiler-rt/lib/xray/xray_utils.cc:45
+
+void printToStdErr(const char *Buffer) XRAY_NEVER_INSTRUMENT {
+  __sanitizer_log_write(Buffer, strlen(Buffer));
----------------
This function is actually dead.  Just remove it.


================
Comment at: compiler-rt/lib/xray/xray_utils.cc:136
+#else // SANITIZER_FUCHSIA
 void printToStdErr(const char *Buffer) XRAY_NEVER_INSTRUMENT {
   fprintf(stderr, "%s", Buffer);
----------------
dead


================
Comment at: compiler-rt/lib/xray/xray_x86_64.cc:113
+uint64_t getTSCFrequency() XRAY_NEVER_INSTRUMENT {
+    return static_cast<uint64_t>(zx_ticks_per_second());
+}
----------------
Drop the cast.  zx_ticks_t is just uint64_t and if it changed so this conversion did something, we'd want the warnings about it.
Use _zx_* names.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D52162