This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Add missing -L to libomptarget tests
ClosedPublic

Authored by jdenny on Apr 27 2023, 4:09 PM.

Details

Summary

Without this patch, if an incompatible libomptarget.so is present in a
system directory, such as /usr/lib64, check-openmp fails many
libomptarget tests with linking errors. The problem appears to have
started at D129875, which landed as dc52712a0632. This patch extends
the libomptarget test suite config with a -L for the current build
directory of libomptarget.so.

Diff Detail

Event Timeline

jdenny created this revision.Apr 27 2023, 4:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2023, 4:09 PM
jdenny requested review of this revision.Apr 27 2023, 4:09 PM
jhuber6 accepted this revision.Apr 27 2023, 4:20 PM

Surprised this didn't come up sooner.

This revision is now accepted and ready to land.Apr 27 2023, 4:20 PM
JonChesterfield accepted this revision.Apr 27 2023, 4:56 PM

Interesting. I wonder what libraries were being picked up before - maybe from the cmake install directory?

L is simpler than linking the specific libraries individually. There's a failure mode where the libraries aren't in the expected directory and other libraries with the same name are found elsewhere, but hopefully cmake errors observably if the library didn't build.

This revision was automatically updated to reflect the committed changes.

Interesting. I wonder what libraries were being picked up before - maybe from the cmake install directory?

Do you mean before dc52712a0632? My understanding is that libomptarget.so was located in a different build directory then, so the existing -L flags in the lit config were sufficient.

jdenny added a comment.EditedMay 1 2023, 10:27 AM

Interesting. I wonder what libraries were being picked up before - maybe from the cmake install directory?

Revisiting this today, I think that "before" might mean "before the patch under review". When Clang invokes ld, it first passes -L from its own command line (in this case specified in the lit config), then -L for system directories, and then -L for the build's lib directory. That should make it clear why it normally works fine if there's no libomptarget.so in a system directory, and otherwise the Clang command line needs a -L for the build's lib directory to avoid it.