Page MenuHomePhabricator

[clang] Search runtimes build tree for openmp runtime
Needs RevisionPublic

Authored by JonChesterfield on May 5 2021, 11:52 AM.



D96248 added support for finding the openmp devicertl bitcode in the clang
lib directory where it is installed, or in the LIBRARY_PATH environment.

When testing openmp from a build using ENABLE_RUNTIMES, the corresponding
bitcode may be found under runtimes/runtimes-bins. This patch adds that to
the list of places clang looks, at lower priority then the others.

Diff Detail

Unit TestsFailed

2,460 msx64 debian > libarcher.races::lock-unrelated.c
Script: -- : 'RUN: at line 13'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests -I /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/src -L /mnt/disks/ssd0/agent/llvm-project/build/lib -Wl,-rpath,/mnt/disks/ssd0/agent/llvm-project/build/lib /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/races/lock-unrelated.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/deflake.bash /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp 2>&1 | tee /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp.log | /mnt/disks/ssd0/agent/llvm-project/build/./bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/races/lock-unrelated.c

Event Timeline

JonChesterfield requested review of this revision.May 5 2021, 11:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 5 2021, 11:52 AM
JonChesterfield added a comment.EditedMay 5 2021, 2:46 PM

I think the lit test suite is intended to set environment variables such that this is not necessary when running the tests.

edit: This patch resulted from some confusion on my part. Given a list test that fails, copy&paste the failing RUN line into a shell fails to find the device runtime, because the environment setup done by lit is not reflected in the printed RUN line.

That seems to be a drawback from using environment variables to tie the test cases together. We could emit LIBRARY_PATH= lines in front of RUN, but that is probably a cross-OS hazard.

tianshilei1992 requested changes to this revision.May 11 2021, 8:05 AM

From my perspective, this is not a good direction. No need to bother Clang driver for one case of testing. LIBRARY_PATH needs to be set correctly by lit instead of hardcoded the logic in the driver.

This revision now requires changes to proceed.May 11 2021, 8:05 AM
JonChesterfield added a comment.EditedMay 11 2021, 8:20 AM

More compelling than the testing case is that openmp is presently unusable without setting ld_library_path. This is part of fixing that.

Edit: got this confused with one of the other patches. This one is only valuable if the other patches to drop library path land.

Unlike others, such as /usr/local/lib or /usr/local/cuda, it is not a standard place. It doesn't make too much sense to hardcode it in the driver.

I'm not so sure about that. With the other stuff in flight, it would let us build openmp apps from the clang build directory. That's useful for quicker build/debug cycles, as well as for lit tests that can be run by copying into a shell. At zero runtime cost when the library is found elsewhere, and trivial extra code in the driver.

JonChesterfield edited the summary of this revision. (Show Details)Tue, May 25, 3:18 PM