This is an archive of the discontinued LLVM Phabricator instance.

[GTest] Change detection of libpthread
ClosedPublic

Authored by nemanjai on Apr 13 2022, 10:11 AM.

Details

Reviewers
EricWF
amyk
Group Reviewers
Restricted Project
Summary

We currently use CMake's find_library function to detect whether libpthread exists on the system to determine if pthread should be added on the link step. However, there are configurations in which CMake's path checking fails to find the library even though the toolchain has it.

One such case is with Clang 14.0.0 on PowerPC. Due to a recent change, the build puts libc++ and related libraries in a subdirectory that appears to depend on the default target triple. CMake then uses that subdirectory to determine the architecture and adds that name to its search paths. However, the triple for the system GNU toolchain is different so CMake fails to find it.
Namely, Clang 14.0.0's default target triple and the subdirectory name is powerpc64le-unknown-linux-gnu whereas the system GNU toolchain has powerpc64le-linux-gnu. Clang's driver has no trouble finding either the GNU includes/libraries or Clang's own. But CMake seems to get this wrong.

The net result of this is that we can't do a shared libraries build of ToT with Clang 14.0.0.

This patch proposes using HAVE_LIBPTHREAD which CMake seems to determine by compiling a test file with -lpthread (or perhaps -pthread, I can't really get CMake to tell me how it is figuring this out). If that variable tells CMake that the build compiler accepts the pthread option, it seems reasonable to depend on that variable to determine if we should add it to the link step when building the llvm_gtest library.

Diff Detail

Event Timeline

nemanjai created this revision.Apr 13 2022, 10:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 10:11 AM
nemanjai requested review of this revision.Apr 13 2022, 10:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 10:11 AM
amyk accepted this revision as: amyk.EditedAug 8 2022, 9:35 AM
amyk added a subscriber: amyk.

This change overall LGTM. This is also needed to build LLVM 15 for Linux on Power.
@EricWF Did you have any comments/concerns with this patch?

This revision is now accepted and ready to land.Aug 8 2022, 9:35 AM
nemanjai closed this revision.Aug 22 2022, 11:07 AM

I forgot to add the differential revision to the commit message.

Committed in https://reviews.llvm.org/rG8537a99b2c1d