[PATCH] D41121: [sanitizer] Introduce a vDSO aware timing function

[Kamil Rytarowski via Phabricator via llvm-commits]

krytarowski added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:747
+  return (u64)ts.tv_sec * (1000ULL * 1000 * 1000) + ts.tv_nsec;
+}
+
----------------
alekseyshl wrote:
> Now it seems that it would be easier to parse if we separate implementations completely:
> 
> 
>   #if SANITIZER_LINUX && !SANITIZER_GO
> 
>   ...all the defs...
> 
>   u64 MonotonicNanoTime() {
>     timespec ts;
>     if (CanUseVDSO()) {
>       if (&real_clock_gettime)
>         real_clock_gettime(CLOCK_MONOTONIC, &ts);
>       else
>         clock_gettime(CLOCK_MONOTONIC, &ts);
>     } else {
>       internal_clock_gettime(CLOCK_MONOTONIC, &ts);
>     }
>     return (u64)ts.tv_sec * (1000ULL * 1000 * 1000) + ts.tv_nsec;
>   }
> 
>   #else // !(SANITIZER_LINUX && !SANITIZER_GO)
> 
>   u64 MonotonicNanoTime() {
>     timespec ts;
>     internal_clock_gettime(CLOCK_MONOTONIC, &ts);
>     return (u64)ts.tv_sec * (1000ULL * 1000 * 1000) + ts.tv_nsec;
>   }
> 
>   #endif // SANITIZER_LINUX && !SANITIZER_GO
> 
> 
> 
> Also can do 
> 
>   u64 MonotonicNanoTime() {
>     timespec ts;
>     MonotonicNanoTimeImpl(&ts)
>     return (u64)ts.tv_sec * (1000ULL * 1000 * 1000) + ts.tv_nsec;
>   }
> 
> and define inlined MonotonicNanoTimeImpls for each case, but that is up to you.
This split looks good!


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D41121