[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-16 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D75811#1923281 , @tambre wrote: > Your help here and over on CMake's side has been very helpful. Thank you! > I'll @ you on CMake's side if I need any help while working on CUDA support. > Hopefully you won't mind. :) No problem

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-14 Thread Raul Tambre via Phabricator via cfe-commits
tambre added a comment. In D75811#1923278 , @csigg wrote: > > I'll be adding a `CUDA_ROOT` option to CMake that will be passed to clang > > as `--cuda-path`. > > I'm not familiar with CMake and whether that option is picked up from an > environment varia

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-14 Thread Raul Tambre via Phabricator via cfe-commits
tambre added a comment. Your help here and over on CMake's side has been very helpful. Thank you! I'll @ you on CMake's side if I need any help while working on CUDA support. Hopefully you won't mind. :) I'm progressing on this and hope to have initial support in a mergeable state within two we

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-14 Thread Christian Sigg via Phabricator via cfe-commits
csigg added a comment. > I've gone with the approach of trying the architectures in the most recent > non-deprecated order – sm_52, sm_30. I'm curious why you added sm_52 (I'm currently writing bazel rules for better CUDA support, and I'm using just sm_30 because that's been nvcc's default for

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-14 Thread Christian Sigg via Phabricator via cfe-commits
csigg added a comment. > I'll be adding a `CUDA_ROOT` option to CMake that will be passed to clang as > `--cuda-path`. I'm not familiar with CMake and whether that option is picked up from an environment variable, but on Windows that environment variable that the CUDA installer sets is `CUDA_P

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-12 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D75811#1919368 , @tambre wrote: > After some work on my CMake changes, Clang detection as a CUDA compiler works > and I can compile CUDA code. \o/ Nice! Having cmake supporting clang as a cuda compiler out of the box would be re

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-12 Thread Raul Tambre via Phabricator via cfe-commits
tambre added a comment. Thank you for the long and detailed explanation. It's been of great help! I've gone with the approach of trying the architectures in the most recent non-deprecated order – sm_52, sm_30. A problem with bumping the default architecture would have been that there are alre

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-09 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D75811#1913148 , @tambre wrote: > > Magically changing compiler target based on something external to compiler > > is a bad idea IMO. I would expect a compilation with exactly the same > > compiler options to do exactly the same t

Re: [PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-09 Thread Justin Lebar via cfe-commits
From the peanut gallery: Perhaps something like --cuda_arch=min_supported would solve your problem while still meeting tra's request not to change behavior of the compiler based on something external. On Mon, Mar 9, 2020 at 12:58 PM Raul Tambre via Phabricator wrote: > > tambre added a comment. >

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-09 Thread Raul Tambre via Phabricator via cfe-commits
tambre added a comment. > Magically changing compiler target based on something external to compiler is > a bad idea IMO. I would expect a compilation with exactly the same compiler > options to do exactly the same thing. If we magically change default target, > that will not be the case. It'd

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-09 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. I'm not sure that's the problem worth solving. Magically changing compiler target based on something external to compiler is a bad idea IMO. I would expect a compilation with exactly the same compiler options to do exactly the same thing. If we magically change default targ

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-07 Thread Raul Tambre via Phabricator via cfe-commits
tambre added inline comments. Comment at: clang/lib/Driver/Driver.cpp:2579 + if (Version == CudaVersion::UNKNOWN) { +C.getDriver().Diag(clang::diag::err_drv_cuda_unknown_version) +<< VersionString; This error is hit when simply running `c

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-07 Thread Raul Tambre via Phabricator via cfe-commits
tambre updated this revision to Diff 248949. tambre added a comment. Add missing error message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75811/new/ https://reviews.llvm.org/D75811 Files: clang/include/clang/Basic/Cuda.h clang/include/clang

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-07 Thread Raul Tambre via Phabricator via cfe-commits
tambre added inline comments. Comment at: clang/lib/Driver/Driver.cpp:642 + *this, CudaTriple, *HostTC, C.getInputArgs(), OFK); + C.getArgs().AddJoinedArg( + nullptr, getOpts().getOption(options::OPT_cuda_version_EQ), This isn't very pr

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-07 Thread Raul Tambre via Phabricator via cfe-commits
tambre created this revision. tambre added reviewers: tra, jlebar. Herald added a project: clang. Herald added a subscriber: cfe-commits. tambre added inline comments. Comment at: clang/lib/Driver/Driver.cpp:642 + *this, CudaTriple, *HostTC, C.getInputArgs(), OFK); +