JonChesterfield planned changes to this revision.
JonChesterfield added a comment.
Yeah, we probably can't leave the installed program with a search path only
used for testing. I'm planning to revisit this to use
libomptarget_amdgcn_bc_path instead of LIBRARY_PATH for testing, then drop the
LIB
ye-luo added a comment.
It seems that this path is baked in to clang executable even after make
install. I'm not convinced this is the right direction.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101935/new/
https://reviews.llvm.org/D101935
___
JonChesterfield added a comment.
At present libomptarget_amdgcn_bc_path and nvptx need to be a path to a file.
If we relax that to accept a path to a directory in which the corresponding
file is found, then we can use that argument in place of LIBRARY_PATH from the
test scripts. That will let t
JonChesterfield added a comment.
I'm not so sure about that. With the other stuff in flight, it would let us
build openmp apps from the clang build directory. That's useful for quicker
build/debug cycles, as well as for lit tests that can be run by copying into a
shell. At zero runtime cost whe
tianshilei1992 added a comment.
Unlike others, such as `/usr/local/lib` or `/usr/local/cuda`, it is not a
standard place. It doesn't make too much sense to hardcode it in the driver.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101935/new/
https:
JonChesterfield added a comment.
More compelling than the testing case is that openmp is presently unusable
without setting ld_library_path. This is part of fixing that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101935/new/
https://reviews.llv
tianshilei1992 requested changes to this revision.
tianshilei1992 added a comment.
This revision now requires changes to proceed.
From my perspective, this is not a good direction. No need to bother Clang
driver for one case of testing. `LIBRARY_PATH` needs to be set correctly by lit
instead of
JonChesterfield added a comment.
I think the lit test suite is intended to set environment variables such that
this is not necessary.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101935/new/
https://reviews.llvm.org/D101935
_
JonChesterfield created this revision.
JonChesterfield added reviewers: jdoerfert, pdhaliwal, ronlieb, grokos,
tianshilei1992.
Herald added subscribers: guansong, yaxunl.
JonChesterfield requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: cla
