[PATCH] D40828: [XRay][compiler-rt] Implement XRay Basic Mode Filtering

[Eizan Miyamoto via Phabricator via llvm-commits]

eizan added inline comments.


================
Comment at: compiler-rt/lib/xray/xray_inmemory_log.cc:48
+  uint16_t Type;
+  uint8_t CPU;
+  uint8_t Padding;
----------------
is 8 bits enough for a CPU number much longer? E.g., getcpu() on Linux returns an unsigned integer for a cpu number.


================
Comment at: compiler-rt/lib/xray/xray_inmemory_log.cc:56
 struct alignas(64) ThreadLocalData {
-  XRayRecord *InMemoryBuffer = nullptr;
+  void *InMemoryBuffer = nullptr;
   size_t BufferSize = 0;
----------------
Why did you change this from XRayRecord * to void *?


================
Comment at: compiler-rt/lib/xray/xray_inmemory_log.cc:123
+    }
+    TLD.TID = syscall(SYS_gettid);
     pthread_setspecific(PThreadKey, &TLD);
----------------
Are there any plans to make this useable on other operating systems? What about pthread_self() instead?


================
Comment at: compiler-rt/lib/xray/xray_inmemory_log.cc:173
+
+  uint8_t CPU = 0;
+  uint64_t TSC = ReadTSC(CPU);
----------------
Why are you hard-coding this to 0 instead of getting the CPU number that the thread is running on?


https://reviews.llvm.org/D40828