[PATCH] D29703: [XRay] [compiler-rt] Allow logging the first argument of a function call.

[Dean Michael Berris via Phabricator via llvm-commits]

dberris requested changes to this revision.
dberris added inline comments.
This revision now requires changes to proceed.


================
Comment at: include/xray/xray_interface.h:26-27
+  TAIL = 2,
+  // 3 unused and available (custom logging?)
+  LOG_ARGS_ENTRY = 4,
+};
----------------
Use 3 instead (we don't have anything using 3 yet)?


================
Comment at: lib/xray/xray_AArch64.cc:124-126
+void __xray_ArgLoggerEntry() XRAY_NEVER_INSTRUMENT {
+  // FIXME: this will have to be implemented in the trampoline assembly file
+}
----------------
I don't think this will work -- this will be mangled differently from an `extern "C" { ... }` function, which this may have to be to make the symbol consistent. This means you'd have to define that symbol outside the `__xray` namespace, and mark it `exter "C" {...}`. As in:

```

} // namespace __xray

extern "C" {
void __xray_ArgLoggerEntry() XRAY_NEVER_INSTRUMENT {
  // FIXME: ...
}
} 
```


================
Comment at: lib/xray/xray_arm.cc:160-162
+void __xray_ArgLoggerEntry() XRAY_NEVER_INSTRUMENT {
+  // FIXME: this will have to be implemented in the trampoline assembly file
+}
----------------
Same here.


================
Comment at: lib/xray/xray_interface.cc:217
+{
+  XRayArgLogger.store(Handler, std::memory_order_relaxed);
+}
----------------
I think it would be better to make this store a release, for a stronger guarantee. 


https://reviews.llvm.org/D29703