Skip to content

[OpenMP] Fix test after updating library search paths #83573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions clang/lib/Driver/ToolChains/CommonArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2767,10 +2767,6 @@ void tools::addOpenMPDeviceRTL(const Driver &D,
const ToolChain &HostTC) {
SmallVector<StringRef, 8> LibraryPaths;

// Check all of the standard library search paths used by the compiler.
for (const auto &LibPath : HostTC.getFilePaths())
LibraryPaths.emplace_back(LibPath);

// Add user defined library paths from LIBRARY_PATH.
std::optional<std::string> LibPath =
llvm::sys::Process::GetEnv("LIBRARY_PATH");
Expand All @@ -2782,6 +2778,10 @@ void tools::addOpenMPDeviceRTL(const Driver &D,
LibraryPaths.emplace_back(Path.trim());
}

// Check all of the standard library search paths used by the compiler.
for (const auto &LibPath : HostTC.getFilePaths())
LibraryPaths.emplace_back(LibPath);

OptSpecifier LibomptargetBCPathOpt =
Triple.isAMDGCN() ? options::OPT_libomptarget_amdgpu_bc_path_EQ
: options::OPT_libomptarget_nvptx_bc_path_EQ;
Expand Down
11 changes: 0 additions & 11 deletions clang/test/Driver/openmp-offload-gpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,6 @@

/// ###########################################################################

/// Check that the warning is thrown when the libomptarget bitcode library is not found.
/// Libomptarget requires sm_52 or newer so an sm_52 bitcode library should never exist.
// RUN: not %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
// RUN: -Xopenmp-target -march=sm_52 --cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
// RUN: -fopenmp-relocatable-target -save-temps %s 2>&1 \
// RUN: | FileCheck -check-prefix=CHK-BCLIB-WARN %s

// CHK-BCLIB-WARN: no library 'libomptarget-nvptx-sm_52.bc' found in the default clang lib directory or in LIBRARY_PATH; use '--libomptarget-nvptx-bc-path' to specify nvptx bitcode library
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When removing this test you suddenly lose code coverage for the err_drv_omp_offload_target_missingbcruntime diagnostic. That seems a bit unfortunate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not overly concerned, we could change it to be a no_such_file error instead. Realistically this was just a mess because whoever set it up initially didn't put it into a place that's guaranteed to be looked at first, so instead they just forced it to only look in once place.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Just wanted to raise the concern. Anyway this patch would help, to avoid failures in our downstream buildbots (and I wouldn't need to workaround those problems downstream).
So LGTM!


/// ###########################################################################

/// Check that the error is thrown when the libomptarget bitcode library does not exist.
// RUN: not %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
// RUN: -Xopenmp-target -march=sm_52 --cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
Expand Down