[PATCH] D14604: [tsan] Filter OS X architectures for unit testing

[Kuba Brecka via llvm-commits]

kubabrecka added inline comments.

================
Comment at: lib/tsan/tests/CMakeLists.txt:38
@@ +37,3 @@
+  if(APPLE)
+    darwin_filter_host_archs(TSAN_SUPPORTED_ARCH TSAN_TEST_ARCH)
+  endif()
----------------
glider wrote:
> kubabrecka wrote:
> > glider wrote:
> > > I'm not familiar with CMake infrastructure enough but I think darwin_filter_host_archs isn't the right way to say "remove ARM64 from the list".
> > > Can we just explicitly set TSAN_TEST_ARCH to x86_64 on Darwin?
> > `darwin_filter_host_archs` is already used for this purpose in `asan/tests/` and `sanitizer_common/tests/`.  It's actually doing a little more work, namely keeping both x86_64 and x86_64h in case your system is a Haswell+ machine.
> > 
> > But I'm actually completely fine with just doing `set(TSAN_TEST_ARCH x86_64)` here, if you still want me to.
> Sorry, I cannot comment on whether we want x86_64h or not.
> Does that mean that we'll be running the tests twice on the Haswell+ machines?
> I think it's good to be consistent with the rest of compiler-rt, but I don't understand why this is done there in the first place.
> If you know this is needed, I'm fine with keeping darwin_filter_host_archs
Yes, this is consistent with the rest of compiler-rt.  We're already running both lit tests and unit tests of ASan and sanitizer-common for x86_64 and x86_64h, if your system is post-Haswell.  That's because we also *build* two slices into the ASan dylib...

    $ lipo -info ./lib/clang/3.8.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib 
    Architectures in the fat file: ./lib/clang/3.8.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib are: i386 x86_64 x86_64h

...so we want to test them both.


http://reviews.llvm.org/D14604