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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2a581710c194: [openmp] No longer use LIBRARY_PATH to find devicertl (authored by JonChesterfield). Repository: rG LLVM Github Monorepo CHANGES SI

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

JonChesterfield updated this revision to Diff 371614. JonChesterfield added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109061/new/ https://reviews.llvm.org/D109061 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/

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

tianshilei1992 accepted this revision. tianshilei1992 added a comment. This revision is now accepted and ready to land. LGTM. It's good to go if the test is green. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109061/new/ https://reviews.llvm.org/D

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

JonChesterfield updated this revision to Diff 371605. JonChesterfield added a comment. - Add test checking LIBRARY_PATH is used Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109061/new/ https://reviews.llvm.org/D109061 Files: clang/lib/Driver/To

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

tianshilei1992 added inline comments. Comment at: clang/test/Driver/openmp-offload-gpu.c:153 -/// bitcode library and add it to the LIBRARY_PATH. -// RUN: env LIBRARY_PATH=%S/Inputs/libomptarget %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \ -// RUN: -Xop

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

JonChesterfield added inline comments. Comment at: clang/test/Driver/openmp-offload-gpu.c:153 -/// bitcode library and add it to the LIBRARY_PATH. -// RUN: env LIBRARY_PATH=%S/Inputs/libomptarget %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \ -// RUN: -Xo

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

tianshilei1992 added inline comments. Comment at: clang/test/Driver/openmp-offload-gpu.c:153 -/// bitcode library and add it to the LIBRARY_PATH. -// RUN: env LIBRARY_PATH=%S/Inputs/libomptarget %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \ -// RUN: -Xop

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

JonChesterfield updated this revision to Diff 371450. JonChesterfield added a comment. - Use regex for windows path compat Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109061/new/ https://reviews.llvm.org/D109061 Files: clang/lib/Driver/ToolCha

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

JonChesterfield updated this revision to Diff 371419. JonChesterfield added a comment. - Keep LIBRARY_PATH search, as fallback if local file is missing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109061/new/ https://reviews.llvm.org/D109061 File

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

tianshilei1992 added a comment. In D109061#2989363 , @JonChesterfield wrote: > @tianshilei1992 can we agree that the tests must point to a specific > deviceRTL, and not dig one out of LIBRARY_PATH? That can be factored out of > this patch to make the t

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

JonChesterfield added a comment. Discussed at weekly call. Going to go with the priority order: 1/ commandline argument 2/ look next to clang 3/ LIBRARY_PATH 4/ error That means people who have installed clang+openmp together have something that works out of the box, and they can override the de

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

JonChesterfield added a comment. @tianshilei1992 can we agree that the tests must point to a specific deviceRTL, and not dig one out of LIBRARY_PATH? That can be factored out of this patch to make the tests always use exactly the library that was just built, and also makes them easier to run ou

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

tianshilei1992 added a comment. In order to tackle the issues, I think we can do it this way: put the clang default lib ahead of `LIBRARY_PATH`. This can make sure `clang` always uses the bitcode library that comes with it, and if it is not there, it also has the ability to find a potential one

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

JonChesterfield requested review of this revision. JonChesterfield added a comment. Sending back to review now that the test updates are included and LIBRARY_PATH removed from the test setup entirely Comment at: clang/test/Driver/openmp-offload-gpu.c:151 /// Check that the

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

JonChesterfield updated this revision to Diff 370087. JonChesterfield added a comment. Herald added subscribers: kerbowa, nhaehnle, jvesely. - Update tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109061/new/ https://reviews.llvm.org/D109061

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

JonChesterfield updated this revision to Diff 370047. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109061/new/ https://reviews.llvm.org/D109061 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp openmp/libomptarget/test/lit.cfg Index: openmp/l

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

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 --li

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

tianshilei1992 added a comment. In D109061#2977714 , @JonChesterfield wrote: > Using LIBRARY_PATH to find the bitcode is a reasonable interpretation of > LIBRARY_PATH but it's a disaster in terms of using clang on a system that has > LIBRARY_PATH point

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

JonChesterfield added a comment. Using LIBRARY_PATH to find the bitcode is a reasonable interpretation of LIBRARY_PATH but it's a disaster in terms of using clang on a system that has LIBRARY_PATH pointing at some other version of clang. Rpath will be ignored by the bitcode lib handling. How d

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

tianshilei1992 added a comment. From my perspective, using `LIBRARY_PATH` to find bitcode library conforms with convention because the library is linked in a linkage during compilation time, where `LIBRARY_PATH` is used by the linker. If we don't want to mess up test results, the patch to force

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

tianshilei1992 added a comment. In D109061#2977690 , @JonChesterfield wrote: > Nope, works the same as it used to - looks relative to clang. However I think > it's broken some CI machines Well, it also breaks when OpenMP is built standalone. Reposito

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

JonChesterfield added a comment. Nope, works the same as it used to - looks relative to clang. However I think it's broken some CI machines Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109061/new/ https://reviews.llvm.org/D109061 ___

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7a228f872fbb: [openmp] No longer use LIBRARY_PATH to find devicertl (authored by JonChesterfield). Repository: rG LLVM Github Monorepo CHANGES SI

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

tianshilei1992 added a comment. I suppose it require users to add a command line argument every time they want to build a program? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109061/new/ https://reviews.llvm.org/D109061

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

JonChesterfield updated this revision to Diff 370024. JonChesterfield added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109061/new/ https://reviews.llvm.org/D109061 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp openmp

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

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109061/new/ https://reviews.llvm.org/D109061 _

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

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1701 StringRef ArchPrefix = Triple.isAMDGCN() ? "amdgcn" : "nvptx"; + std::string LibOmpTargetName = "libomptarget-" + BitcodeSuffix.str() + ".bc"; This part is co

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

JonChesterfield created this revision. JonChesterfield added reviewers: jdoerfert, grokos, tianshilei1992. Herald added subscribers: guansong, yaxunl. JonChesterfield requested review of this revision. Herald added subscribers: openmp-commits, cfe-commits, sstefan1. Herald added projects: clang, Op