[PATCH] D21612: [compiler-rt] [XRay] Basic initialization and flag definition for XRay runtime

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: lib/xray/xray_interface.cc:36-55
@@ +35,22 @@
+  // actual work.
+  uint64_t Rrdi, Rrax, Rrdx, Rrsi, Rrcx, Rr8, Rr9, Rrbp;
+  __asm__ __volatile__("mov %%rbp, %0" : "=m"(Rrbp));
+  __asm__ __volatile__("mov %%rdi, %0" : "=m"(Rrdi));
+  __asm__ __volatile__("mov %%rax, %0" : "=m"(Rrax));
+  __asm__ __volatile__("mov %%rdx, %0" : "=m"(Rrdx));
+  __asm__ __volatile__("mov %%rsi, %0" : "=m"(Rrsi));
+  __asm__ __volatile__("mov %%rcx, %0" : "=m"(Rrcx));
+  __asm__ __volatile__("mov %%r8, %0" : "=m"(Rr8));
+  __asm__ __volatile__("mov %%r9, %0" : "=m"(Rr9));
+
+  // FIXME: Handle async signal safety, and prevent recursive calls.
+  auto Fn = __xray::XRayPatchedFunction.load(std::memory_order_acquire);
+  if (Fn != nullptr) {
+    int32_t FunctionID;
+    __asm__("mov %%r10d, %0" : "=g"(FunctionID) : : "%r10");
+    (*Fn)(FunctionID, XRayEntryType::ENTRY);
+  }
+
+  // Then restore the registers before returning.
+  __asm__ __volatile__("mov %0,%%r9" : : "m"(Rr9) : "%r9");
+  __asm__ __volatile__("mov %0,%%r8" : : "m"(Rr8) : "%r8");
----------------
I actually think this is important enough to get right the first time. Inline asm has scarred me enough times already.

================
Comment at: lib/xray/xray_interface.cc:38
@@ +37,3 @@
+  __asm__ __volatile__("mov %%rbp, %0" : "=m"(Rrbp));
+  __asm__ __volatile__("mov %%rdi, %0" : "=m"(Rrdi));
+  __asm__ __volatile__("mov %%rax, %0" : "=m"(Rrax));
----------------
OK, but the prologue will push RBP if it is clobbered anywhere in this function, and if it isn't, the prologue of 'Fn' will preserve it.

================
Comment at: lib/xray/xray_interface.cc:62
@@ +61,3 @@
+  __asm__ __volatile__("mov %0,%%rdi" : : "m"(Rrdi) : "%rdi");
+  __asm__ __volatile__("mov %0,%%rbp" : : "m"(Rrbp) : "%rbp");
+}
----------------
What about saving and restoring XMM registers? Those are used for parameters and return values, and are usually considered to be scratch. Is it assumed that XRay authors will carefully audit their code to avoid SSE instructions? Even memcpy uses SSE these days, though.

================
Comment at: lib/xray/xray_interface.cc:69-70
@@ +68,4 @@
+  uint64_t Rrax, Rrdx, Rrbp;
+  __asm__ __volatile__("mov %%rax, %0" : "=m"(Rrax));
+  __asm__ __volatile__("mov %%rdx, %0" : "=m"(Rrdx));
+  __asm__ __volatile__("mov %%rbp, %0" : "=m"(Rrbp));
----------------
I guess you only save and restore RAX/RDX since those are use for return values. I'm sure other calling conventions can break this assumption, but we can leave it this way for now if you want.


https://reviews.llvm.org/D21612