[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

Michał Górny via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 22 00:33:02 PST 2018


mgorny marked an inline comment as done.
mgorny added inline comments.


================
Comment at: lib/xray/xray_defs.h:22
 
+#if !SANITIZER_NETBSD
+#define XRAY_TLS_ALIGNAS(x) alignas(x)
----------------
dberris wrote:
> krytarowski wrote:
> > I would switch the order, in order to remove unneeded negation.
> > 
> > ```
> > #if SANITIZER_NETBSD
> > ...
> > #else
> > ...
> > #endif
> > ```
> > 
> > `#define XRAY_HAS_TLS_ALIGNAS 0` is not needed as `#if XRAY_HAS_TLS_ALIGNAS` will evaluate to false anyway and `#define XRAY_HAS_TLS_ALIGNAS 1` is equivalent to `#define XRAY_HAS_TLS_ALIGNAS`. But it's just a matter of style.
> FWIW, I agree to this suggestion. I also prefer:
> 
> ```
> #if SANITIZER_NETBSD
> ...
> #else
> ...
> ```
> 
> But I do like the explicit definition of `XRAY_HAS_TLS_ALIGNAS`.
I've chosen the explicit method to match what's already done e.g. in `sanitizer_platform.h`. I'll swap the order.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56000/new/

https://reviews.llvm.org/D56000





More information about the llvm-commits mailing list