[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur created this revision. Meinersbur added reviewers: jdoerfert, RaviNarayanaswamy, ye-luo, jhuber6, phosek, grokos, tambre, rnk, sylvestre.ledru, gtbercea, tra, yaxunl, Hahnfeld, jdenny. Meinersbur added a project: clang. Herald added subscribers: openmp-commits, dang, ormris, mgorny. He

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Artem Belevich via Phabricator via cfe-commits
tra requested changes to this revision. tra added a comment. This revision now requires changes to proceed. One concern I have is that the path we configure during clang's build is not necessarily the right choice for the user of clang we build. It's likely that the clang in the end will be used

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In D89974#2347938 , @tra wrote: > One concern I have is that the path we configure during clang's build is not > necessarily the right choice for the user of clang we build. It's likely that > the clang in the end will be used o

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In D89974#2347979 , @Hahnfeld wrote: > In D89974#2347938 , @tra wrote: > >> I think the default should still let clang search for CUDA or require the >> user to provide correct CUDA path.

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D89974#2347979 , @Hahnfeld wrote: > In D89974#2347938 , @tra wrote: > >> One concern I have is that the path we configure during clang's build is not >> necessarily the right choice for the

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur updated this revision to Diff 300046. Meinersbur edited the summary of this revision. Meinersbur added a comment. Re-add accidentally removed space Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89974/new/ https://reviews.llvm.org/D89974

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Raul Tambre via Phabricator via cfe-commits
tambre added a comment. I echo @tra's concerns. Having an option for a vendor to append/prepend a toolkit search location seems useful, but currently this seems more like a workaround for an explicit CUDA toolkit path not being passed to the testsuite. Shortcomings in the testrunner don't seem l

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment. The host's build environment already influences the compiler's defaults, e.g. D88929 . Especially libomptarget heavily uses it, and I would argue that these should be consistent, unless explicitly overwritten. However, I see that Ope

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment. > I think the default should still let clang search for CUDA or require the > user to provide correct CUDA path. "Use CUDA path discovered by CMake at > build time" should be a non-default configuration option if/when it's needed > and appropriate. > Having an opti

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In D89974#2348124 , @jdoerfert wrote: > I thought, right now we would configure clang with a cuda path XYZ If you speak of the current state in the repository, AFAICT, Clang doesn't know about a CUDA installation that CMake may

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. > Shortcomings in the testrunner don't seem like a reason to introduce new > build-time configured default search paths into the driver. +1 The root of the problem is that we rely on CUDA SDK as the external dependency and we have no way to reliably predict where the user

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In D89974#2348176 , @tra wrote: > CUDA path is sort of a global configuration parameter for all CUDA > compilations. Perhaps we should consider allowing the user to specify a CUDA > search path candidate via environment variable

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D89974#2348181 , @Hahnfeld wrote: > Don't we already have this via `PATH`? At least that was my motivation back > then and it worked without problems. We do and it should indeed work for the tests. Setting an environment variable

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. My opinion carries less weight since I don't use CUDA, but I agree with everything Art said. Here's some input, if it helps. I like the `PATH` search for `ptxas` as a way to make things work out of the box as often as possible. I don't like the idea of CMake auto-detecting

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D89974#2348310 , @rnk wrote: > Lastly, if those things don't work, there is the `CCC_OVERRIDE_OPTIONS` > environment variable, so the user always has that escape hatch. A separate > environment variable would also be fine. It remi

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2021-04-28 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur abandoned this revision. Meinersbur added a comment. Abandoned in favor of D101266 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89974/new/ https://reviews.llvm.org/D89974