[PATCH] D109061: [openmp] No longer use LIBRARY_PATH to find devicertl

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

That helps disambiguate. You aren't concerned that this change will break the in tree tests, rather that people who currently rely on LIBRARY_PATH to choose a bitcode library to use with a clang located somewhere else in the filesystem will now need to pass --libomptarget-nvptx-bc-path instead, or put a symlink relative to clang.

That is a huge win for anyone who has LIBRARY_PATH pointing to one toolchain and who wants to be able to use clang from another as it replaces (non-diagnosed) runtime misbehaviour with a compile time error about being unable to find the devicertl. E.g. if they've had to set LIBRARY_PATH to find something their application uses that is unrelated to that clang, perhaps a vendor toolchain.

It's also a moderate win in that it hopefully dissuades people from assuming that any clang can be used with any openmp offloading library, which is very much not true.

It breaks the use case of people who have an exactly matching clang and openmp, that they installed in unrelated places, and use LIBRARY_PATH to stitch back together because they don't have to use it for anything else. They get to use the commandline arguments instead.

I think that's great all around. It makes it much less likely that people will mismatch clang and devicertl, which is good for UX and for our bug handling. If we need to preserve the environment plumbing then let's have a new variable named for the purpose, so that it doesn't collide with LIBRARY_PATH, and still wire tests together with the commandline argument as it lets people copy & paste the failing line into a new terminal to debug it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109061/new/

https://reviews.llvm.org/D109061