[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added a comment. In D143306#4183759 , @tstellar wrote: > Should we apply this patch to the release/16.x branch to create a more smooth > transition since the option has been removed in main? Yes, it will be a good idea to apply this to

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

tstellar added a comment. Should we apply this patch to the release/16.x branch to create a more smooth transition since the option has been removed in main? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143306/new/

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay abandoned this revision. MaskRay added a comment. Abandoned by a revert of D118493 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143306/new/ https://reviews.llvm.org/D143306

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

tstellar added a comment. @JonChesterfield I don't think we should be putting any Fedora specific logic into clang's build system or the driver, if that's what you are suggesting. Fedora can always patch the compiler or install a config file to change the default behavior, even though this is

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

JonChesterfield added a comment. If fedora has a specific set of paths it rejects in DT_RUNPATH - and libomptarget is on one of those by default - and user executables find it there - and we can detect we're building on fedora then we can disable the rpath for fedora installs to system root,

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

jdoerfert added a comment. In D143306#4145603 , @tstellar wrote: > In D143306#4145432 , @MaskRay wrote: > >> This is the point. Specifying a driver option to use >> libc++/libc++abi/libunwind doesn't magically

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

tstellar added a comment. In D143306#4145432 , @MaskRay wrote: > This is the point. Specifying a driver option to use > libc++/libc++abi/libunwind doesn't magically change `DT_RUNPATH`. This is > exactly the behavior a user wants for a system Clang. >

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

jhuber6 added a comment. In D143306#4145482 , @jdoerfert wrote: > @jhuber6 Can you look into the last part, creating such a clang.cfg as part > of the build/install process? > > The fact that rpath is always set is a difference but I doubt it's that

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

jdoerfert added a comment. In D143306#4145465 , @MaskRay wrote: > In D143306#4144541 , @jdoerfert > wrote: > >> I'm worried this makes use of LLVM on HPC machines even harder. That said, >> I'm open to

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added a comment. In D143306#4144541 , @jdoerfert wrote: > I'm worried this makes use of LLVM on HPC machines even harder. That said, > I'm open to suggestions and I am not well versed in all the ways we can make > it work. > Our problem is that

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added a comment. In D143306#4144518 , @JonChesterfield wrote: > Marking this as "no" because as far as I can tell it'll stop anyone running > openmp built from source which constitutes a severe regression and I am > struggling to find

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added a comment. In D143306#4144396 , @JonChesterfield wrote: > So what is this configuration file? Joseph found a Gentoo blog post > https://blogs.gentoo.org/mgorny/2022/10/07/clang-in-gentoo-now-sets-default-runtimes-via-config-file/ > and I

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

jdoerfert added a comment. I'm worried this makes use of LLVM on HPC machines even harder. That said, I'm open to suggestions and I am not well versed in all the ways we can make it work. Our problem is that there are N `libomptarget.so` files and N `libomptarget.nvptx.so` files on a system,

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

JonChesterfield requested changes to this revision. JonChesterfield added a comment. This revision now requires changes to proceed. Marking this as "no" because as far as I can tell it'll stop anyone running openmp built from source which constitutes a severe regression and I am struggling to

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

JonChesterfield added inline comments. Comment at: clang/include/clang/Driver/Options.td:4218-4223 defm openmp_implicit_rpath: BoolFOption<"openmp-implicit-rpath", LangOpts<"OpenMP">, - DefaultTrue, + DefaultFalse, PosFlag, NegFlag, BothFlags<[NoArgumentUnused]>>;

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

JonChesterfield added a comment. So what is this configuration file? Joseph found a Gentoo blog post https://blogs.gentoo.org/mgorny/2022/10/07/clang-in-gentoo-now-sets-default-runtimes-via-config-file/ and I don't have a clang.cfg file in my install dir Repository: rG LLVM Github Monorepo

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

JonChesterfield added a comment. I don't know how this configuration file works. Running clang from the build directory is useful when developing, but to avoid user facing regression we'd need it to run from the install directory. The use case is someone builds trunk or a release on a HPC

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:4218-4223 defm openmp_implicit_rpath: BoolFOption<"openmp-implicit-rpath", LangOpts<"OpenMP">, - DefaultTrue, + DefaultFalse, PosFlag, NegFlag, BothFlags<[NoArgumentUnused]>>;

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

tstellar added a comment. In D143306#4142023 , @JonChesterfield wrote: > I don't mind hugely what mechanism is used but would really like clang++ > -fopenmp foo.cpp to build a program that runs. How can we preserve that > 'works' feature without

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

JonChesterfield added a comment. I don't mind hugely what mechanism is used but would really like clang++ -fopenmp foo.cpp to build a program that runs. How can we preserve that 'works' feature without setting rpath on the binary? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

tstellar added a comment. FWIW, I'm in favor of this patch. System directory rpaths (e.g. /usr/lib64) are not allowed in Fedora Linux. The current default makes building packages with clang+openmp more difficult. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

yaxunl added inline comments. Comment at: clang/include/clang/Driver/Options.td:4218-4223 defm openmp_implicit_rpath: BoolFOption<"openmp-implicit-rpath", LangOpts<"OpenMP">, - DefaultTrue, + DefaultFalse, PosFlag, NegFlag, BothFlags<[NoArgumentUnused]>>;

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

jhuber6 added a comment. If we're removing this feature it would probably make sense to remove the handling / flag altogether as `-fopenmp-implicit-rpath` isn't that much different from `-Wl,-rpath=/path/to/llvm/install/lib`. Overall, the problem is that we want to tie the `libomptarget.so`

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

tianshilei1992 added a comment. I prefer to follow established convention. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143306/new/ https://reviews.llvm.org/D143306 ___ cfe-commits mailing list

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added a comment. Ping for feedback from others Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143306/new/ https://reviews.llvm.org/D143306 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added a comment. In D143306#4104208 , @JonChesterfield wrote: > Added some people I can remember from the original discussion. > > The effect of this patch will be that clang -fopenmp foo.cpp will build an > executable that doesn't know where

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

JonChesterfield added reviewers: tianshilei1992, ye-luo, RaviNarayanaswamy. JonChesterfield added a comment. Added some people I can remember from the original discussion. The effect of this patch will be that clang -fopenmp foo.cpp will build an executable that doesn't know where its runtime

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added a comment. No test probably makes this easy to back port. Eventually there needs to be a test but the original author can do it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143306/new/ https://reviews.llvm.org/D143306

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay created this revision. MaskRay added reviewers: jdoerfert, jhuber6, JonChesterfield, tstellar. Herald added subscribers: guansong, yaxunl. Herald added a project: All. MaskRay requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang.