Page MenuHomePhabricator

[tsan] Add a libc++ and lit testsuite for each ${TSAN_SUPPORTED_ARCH}.
ClosedPublic

Authored by dsanders on Jan 28 2016, 8:00 AM.

Details

Summary

This is a workaround to a problem in the 3.8 release that affects MIPS and
possibly other targets where the default is not supported but a sibling is
supported.

When TSAN_SUPPORTED_ARCH is not empty, cmake currently attempts to build a
tsan'd libcxx as well as test tsan for the default target regardless of whether
the default target is supported or not. This causes problems on MIPS32 since
tsan is supported for MIPS64 but not MIPS32.

This patch causes cmake to only build the libcxx and run the lit test-suite for
archictures in ${TSAN_SUPPORTED_ARCH}

Diff Detail

Event Timeline

dsanders updated this revision to Diff 46277.Jan 28 2016, 8:00 AM
dsanders retitled this revision from to [tsan] Add a libc++ and lit testsuite for each ${TSAN_SUPPORTED_ARCH}..
dsanders updated this object.
dsanders added reviewers: samsonov, hans.
dsanders added subscribers: dvyukov, llvm-commits.

There's a couple things about this patch that I'd like to mention

test/tsan/CMakeLists.txt
21–25

This check is duplicated from the equivalent code in asan and it looks a little odd to me. It makes sense to me that ANDROID is only true for cross-compilation but the '${arch} MATCHES "arm|aarch64"' can be true on native builds despite the comment saying it's only true for cross-compilation.

test/tsan/lit.cfg
44

I don't think this -m64 is needed now that we have config.target_cflags. Do you agree?

Sorry for the early ping but I need to get either this patch or D16583 into rc2 so that the mips builds don't fail on 'make check-all' again.

samsonov accepted this revision.Feb 1 2016, 1:27 PM
samsonov edited edge metadata.

Sorry for delay. LGTM. Thank you for taking care of this!

cmake/Modules/AddCompilerRT.cmake
301

Please commit these changes separately, from http://reviews.llvm.org/D16681

lib/tsan/CMakeLists.txt
207

s/dependencies/libcxx_tsan_deps ?

test/tsan/lit.cfg
44

Yes

This revision is now accepted and ready to land.Feb 1 2016, 1:27 PM
dsanders closed this revision.Feb 2 2016, 7:08 AM

Thanks. Both changes were made in the commit (r259512).

hans edited edge metadata.Feb 12 2016, 10:32 AM

Daniel, did everything here get merged to 3.8? (I think this was committed, got reverted, and then I lost track..)

Hi Hans,

I've just double checked trunk and release_38 and it's present on both. We should merge the tsan half of r260669 to make sure the tsan unit tests get run though.