This is an archive of the discontinued LLVM Phabricator instance.

Differential D96187

[OpenMP] Fix libomptarget tests to find libomp
AbandonedPublic

Authored by jdenny on Feb 5 2021, 4:27 PM.

Details

Reviewers
jdoerfert
grokos
tianshilei1992
RaviNarayanaswamy
JonChesterfield
Summary

Without this patch, libomptarget tests fail to find libomp.so, whether
I configure openmp as part of LLVM_ENABLE_RUNTIMES or
LLVM_ENABLE_PROJECTS.

In the LLVM_ENABLE_PROJECTS case only, it's already possible to work
around the problem by setting LD_LIBRARY_PATH appropriately before
calling "ninja check-libomptarget". However, there was a time when
that wasn't necessary, and it seems like it shouldn't be.

Diff Detail

Event Timeline

jdenny created this revision.Feb 5 2021, 4:27 PM
Herald added subscribers: guansong, yaxunl. · View Herald TranscriptFeb 5 2021, 4:27 PM
jdenny requested review of this revision.Feb 5 2021, 4:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 5 2021, 4:27 PM
Herald added a subscriber: sstefan1. · View Herald Transcript
Harbormaster completed remote builds in B88151: Diff 321894.Feb 5 2021, 4:28 PM
jdoerfert added a reviewer: JonChesterfield.Feb 5 2021, 4:29 PM
tianshilei1992 added inline comments.Feb 5 2021, 4:37 PM
openmp/libomptarget/deviceRTLs/nvptx/test/lit.site.cfg.in
8 ↗(On Diff #321894)

We probably don't need an extra variable for that. config.omp_host_rtl_directory is for libomp if I'm not mistaken.

JonChesterfield resigned from this revision.Feb 5 2021, 5:02 PM

Thank you for looking at this. I can't tell whether the change is right, but it's definitely annoying to need to set env variables to run tests.

jdenny updated this revision to Diff 321904.Feb 5 2021, 5:42 PM
jdenny edited the summary of this revision. (Show Details)

Set LIBOMPTARGET_OPENMP_HOST_RTL_FOLDER for non-standalone builds.

Herald added a subscriber: mgorny. · View Herald TranscriptFeb 5 2021, 5:42 PM
jdenny marked an inline comment as done.Feb 5 2021, 5:42 PM
jdenny added inline comments.
openmp/libomptarget/deviceRTLs/nvptx/test/lit.site.cfg.in
8 ↗(On Diff #321894)

Yep, that's better. Thanks.

tianshilei1992 accepted this revision.Feb 5 2021, 7:03 PM

LGTM

This revision is now accepted and ready to land.Feb 5 2021, 7:03 PM
jdenny abandoned this revision.Feb 5 2021, 7:27 PM
jdenny marked an inline comment as done.

Ouch. I checked out master instead of main. This bug has already been fixed. Sorry for the noise.

Revision Contents

PathSize
openmp/
libomptarget/
1 line

Diff 321904

openmp/libomptarget/CMakeLists.txt

Show First 20 Lines • Show All 69 Lines • ▼ Show 20 Lines
# Definitions for testing, for reuse when testing libomptarget-nvptx. # Definitions for testing, for reuse when testing libomptarget-nvptx.
if(OPENMP_STANDALONE_BUILD) if(OPENMP_STANDALONE_BUILD)
set(LIBOMPTARGET_OPENMP_HEADER_FOLDER "${CMAKE_CURRENT_BINARY_DIR}/../runtime/src" CACHE STRING set(LIBOMPTARGET_OPENMP_HEADER_FOLDER "${CMAKE_CURRENT_BINARY_DIR}/../runtime/src" CACHE STRING
"Path to folder containing omp.h") "Path to folder containing omp.h")
set(LIBOMPTARGET_OPENMP_HOST_RTL_FOLDER "${CMAKE_CURRENT_BINARY_DIR}/../runtime/src" CACHE STRING set(LIBOMPTARGET_OPENMP_HOST_RTL_FOLDER "${CMAKE_CURRENT_BINARY_DIR}/../runtime/src" CACHE STRING
"Path to folder containing libomp.so") "Path to folder containing libomp.so")
else() else()
set(LIBOMPTARGET_OPENMP_HEADER_FOLDER "${CMAKE_CURRENT_BINARY_DIR}/../runtime/src") set(LIBOMPTARGET_OPENMP_HEADER_FOLDER "${CMAKE_CURRENT_BINARY_DIR}/../runtime/src")
set(LIBOMPTARGET_OPENMP_HOST_RTL_FOLDER "${LIBOMP_LIBRARY_DIR}")
endif() endif()
# Build offloading plugins and device RTLs if they are available. # Build offloading plugins and device RTLs if they are available.
add_subdirectory(plugins) add_subdirectory(plugins)
add_subdirectory(deviceRTLs) add_subdirectory(deviceRTLs)
# Add tests. # Add tests.
add_subdirectory(test) add_subdirectory(test)