[PATCH] D101960: [openmp] Drop requirement on library path environment variables

ye-luo added a comment. In D101960#2961133 , @jdoerfert wrote: > There are 3 problems here (ignoring our test setup which should be discussed > separately): > > 1. make sure clang finds libomp.so > 2. make sure libomp.so (or clang?) finds libomptarget.so

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield added a comment. Test setup can be fixed independently (and possibly should be). D102043 is newly simplified. It looks for plugins next to libomptarget.so, which means it can find them even when the application uses a non-transitive method to f

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

jdoerfert added a comment. There are 3 problems here (ignoring our test setup which should be discussed separately): 1. make sure clang finds libomp.so 2. make sure libomp.so (or clang?) finds libomptarget.so 3. make sure libomptartget.so finds the plugins All of which have to work nicely with

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield added a comment. In D101960#2960846 , @protze.joachim wrote: > The lit config has platform-specific rules to build the environmental > variables (including the use of the proper separators). I suggest to used > these values to create th

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

protze.joachim added a comment. In D101960#2960641 , @JonChesterfield wrote: > Pasting `env LD_LIBRARY_PATH=` and `env LIBRARY_PATH` into the printed > commandline, as opposed to through magic, would solve most of my day to day > frustration here. libo

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield added a comment. In D101960#2960657 , @jdoerfert wrote: > Do I understand correctly that adding runpath to libomp.so will help it find > libomptarget.so *and* still allows users to use LD_LIBRARY_PATH to make sure > a different libomptar

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

jdoerfert added a comment. Do I understand correctly that adding runpath to libomp.so will help it find libomptarget.so *and* still allows users to use LD_LIBRARY_PATH to make sure a different libomptarget.so is found? If the above is the case, can't we do the same for clang? Asked differently

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:648 +void tools::addOpenMPRuntimeSpecificRPath(const ToolChain &TC, + const ArgList &Args, JonChesterfield wrote: > protze.joa

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield added a comment. Pasting `env LD_LIBRARY_PATH=` and `env LIBRARY_PATH` into the printed commandline, as opposed to through magic, would solve most of my day to day frustration here. libomp.so and libomptarget.so are in two different directories, LD_LIBRARY_PATH turns out to be c

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

lebedev.ri added a comment. In D101960#2960622 , @jdoerfert wrote: > To summarize the conversation, can we do LD_LIBRARY_PATH overwrites after > this patch or not? If so, I feel everyone is in favor, if not, we need a > different solution. +1 Reposit

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

jdoerfert added a comment. To summarize the conversation, can we do LD_LIBRARY_PATH overwrites after this patch or not? If so, I feel everyone is in favor, if not, we need a different solution. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101960/

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

protze.joachim added a comment. To make debugging of failed tests easier, we could just explicitly include `env LD_LIBRARY_PATH=...` into each run line - by adding it to the `%libomp-run` substitution (and the libomptarget equivalent). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield added a comment. Just ran into this again. It's really annoying that a test fails, and prints a run line, which can be copied into a terminal where it abjectly fails to run because the environment variables aren't set. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:648 +void tools::addOpenMPRuntimeSpecificRPath(const ToolChain &TC, + const ArgList &Args, protze.joachim wrote: > JonChesterf

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

protze.joachim added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:648 +void tools::addOpenMPRuntimeSpecificRPath(const ToolChain &TC, + const ArgList &Args, JonChesterfield wrote: > lebedev.ri

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield added inline comments. Comment at: openmp/libomptarget/src/rtl.cpp:76 + std::string full_plugin_name; + void *handle = dlopen("libomptarget.so", RTLD_NOW); JonChesterfield wrote: > JonChesterfield wrote: > > This logic is cut from D73657 witho

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield updated this revision to Diff 343557. JonChesterfield added a comment. - rework logic for finding libomptarget.so Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101960/new/ https://reviews.llvm.org/D101960 Files: clang/lib/Driver/

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield added a comment. Yes. Libomp and libomptarget are in the same directory, so the rpath/runpath change catches both of them. Libomptarget then needs to find the plugins to load, either through library path or something equivalent to the above. Comment at: clang/

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

lebedev.ri added a comment. Thank you for looking into this! This also fixes the problem of not being able to find `libomp`, right? Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:648 +void tools::addOpenMPRuntimeSpecificRPath(const ToolChain &TC, +

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield added inline comments. Comment at: openmp/libomptarget/src/rtl.cpp:76 + std::string full_plugin_name; + void *handle = dlopen("libomptarget.so", RTLD_NOW); JonChesterfield wrote: > This logic is cut from D73657 without addressing any of the re

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:648 +void tools::addOpenMPRuntimeSpecificRPath(const ToolChain &TC, + const ArgList &Args, Similar to other functions in this

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

JonChesterfield created this revision. JonChesterfield added reviewers: jdoerfert, grokos, ABataev, gregrodgers, RaviNarayanaswamy, pdhaliwal, hfinkel, tlwilmar, AndreyChurbanov, jlpeyton, protze.joachim, ye-luo, tianshilei1992. Herald added subscribers: kerbowa, guansong, yaxunl, mgorny, nhaehnl