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

[Dean Michael Berris via llvm-commits]

dberris 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");
----------------
rnk wrote:
> I actually think this is important enough to get right the first time. Inline asm has scarred me enough times already.
Good call. It didn't turn out to be as bad as I thought, though I did need to reverse-engineer what the compiler does for a bit to get it "right". :)

================
Comment at: lib/xray/xray_interface.cc:37
@@ +36,3 @@
+  uint64_t Rrdi, Rrax, Rrdx, Rrsi, Rrcx, Rr8, Rr9, Rrbp;
+  __asm__ __volatile__("mov %%rbp, %0" : "=m"(Rrbp));
+  __asm__ __volatile__("mov %%rdi, %0" : "=m"(Rrdi));
----------------
echristo wrote:
> Meta comment: Typically inline asm can be a single block rather than N statements of 1 per instruction.
Thanks -- I tried that but I didn't know how to properly write out the operands when I tried, so I split them out. :D

Now this is all in its own file, so it shouldn't be that big of a problem anymore.

================
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");
+}
----------------
rnk wrote:
> 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.
Yeah, that's a tricky one. I've been thinking about this too and I think it's worth saving/restoring those just to be safe. Unfortunately that might mean there's a bit more overhead as far as runtime and space requirements is concerned.

================
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));
----------------
rnk wrote:
> 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.
Added a FIXME in the assembly file to address at a later point.


https://reviews.llvm.org/D21612