[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-28 Thread Justin Lebar via Phabricator via cfe-commits
jlebar added a comment. Pushed for Daniele: To github.com:llvm/llvm-project.git 99d2582164c4..6eb826567af0 main -> main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117137/new/ https://reviews.llvm.org/D117137 ___

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-28 Thread Justin Lebar via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6eb826567af0: [Driver] Add CUDA support for --offload param (authored by dcastagna, committed by jlebar). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11713

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-28 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. In D117137#3274035 , @dcastagna wrote: > @yaxunl Are you OK landing this change as it is, without the check for OS and > environment in getHIPOffloadTargetTriple? > We can follow up with patch that adds checks for in OS and enviro

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-26 Thread Daniele Castagna via Phabricator via cfe-commits
dcastagna added a comment. @yaxunl Are you OK landing this change as it is, without the check for OS and environment in getHIPOffloadTargetTriple? We can follow up with patch that adds checks for in OS and environment in Triple.cpp. Is that what you meant? Repository: rG LLVM Github Monorepo

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-26 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. In D117137#3273330 , @tra wrote: > In D117137#3269365 , @yaxunl wrote: > >> Does that mean only "spirv{64}-unknown-unknown" is acceptable, or >> "spirv{64}-amd-unknown-unknown" is also acc

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-26 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D117137#3269365 , @yaxunl wrote: > Does that mean only "spirv{64}-unknown-unknown" is acceptable, or > "spirv{64}-amd-unknown-unknown" is also acceptable? My point is that `unknown` part of the triple is a catch-all for `anything

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-26 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki added a comment. In D117137#3269365 , @yaxunl wrote: > Does that mean only "spirv{64}-unknown-unknown" is acceptable, or > "spirv{64}-amd-unknown-unknown" is also acceptable? Having a vendor component in the triple seems to be acceptable for t

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-25 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D117137#3268548 , @linjamaki wrote: > SPIR-V target requires that the OS and the environment type is unknown (see > TargetInfo::AllocateTarget and BaseSPIRTargetInfo). The problem is that LLVM's triple parser will set `UnknownVen

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-25 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. In D117137#3268548 , @linjamaki wrote: > SPIR-V target requires that the OS and the environment type is unknown (see > TargetInfo::AllocateTarget and BaseSPIRTargetInfo). The clang would fail to > create a SPIR-V target if there

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-25 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki added a comment. SPIR-V target requires that the OS and the environment type is unknown (see TargetInfo::AllocateTarget and BaseSPIRTargetInfo). The clang would fail to create a SPIR-V target if there is an OS or environment component in the target string known by the Triple. This cou

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-24 Thread Artem Belevich via Phabricator via cfe-commits
tra added a subscriber: linjamaki. tra added inline comments. Comment at: clang/lib/Driver/Driver.cpp:153-155 + if (TT->getArch() == llvm::Triple::spirv64 && + TT->getVendor() == llvm::Triple::UnknownVendor && + TT->getOS() == llvm::Triple::UnknownOS)

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-24 Thread Daniele Castagna via Phabricator via cfe-commits
dcastagna marked an inline comment as done. dcastagna added inline comments. Comment at: clang/lib/Driver/Driver.cpp:153-155 + if (TT->getArch() == llvm::Triple::spirv64 && + TT->getVendor() == llvm::Triple::UnknownVendor && + TT->getOS() == llvm::Triple::UnknownOS) --

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-24 Thread Daniele Castagna via Phabricator via cfe-commits
dcastagna updated this revision to Diff 402657. dcastagna added a comment. Remove unknown-unknown check from HIP offload logic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117137/new/ https://reviews.llvm.org/D117137 Files: clang/include/clang/

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-24 Thread Daniele Castagna via Phabricator via cfe-commits
dcastagna added inline comments. Comment at: clang/lib/Driver/Driver.cpp:153-155 + if (TT->getArch() == llvm::Triple::spirv64 && + TT->getVendor() == llvm::Triple::UnknownVendor && + TT->getOS() == llvm::Triple::UnknownOS) tra wrote: > What's expected

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-24 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments. Comment at: clang/lib/Driver/Driver.cpp:153-155 + if (TT->getArch() == llvm::Triple::spirv64 && + TT->getVendor() == llvm::Triple::UnknownVendor && + TT->getOS() == llvm::Triple::UnknownOS) What's expected to happen if someon

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-21 Thread Daniele Castagna via Phabricator via cfe-commits
dcastagna updated this revision to Diff 402031. dcastagna added a comment. Rebase on ToT Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117137/new/ https://reviews.llvm.org/D117137 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-20 Thread Daniele Castagna via Phabricator via cfe-commits
dcastagna updated this revision to Diff 401813. dcastagna added a comment. Fix invalid-offload-options.cpp test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117137/new/ https://reviews.llvm.org/D117137 Files: clang/include/clang/Basic/Diagnosti

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-20 Thread Daniele Castagna via Phabricator via cfe-commits
dcastagna added a comment. In D117137#3259276 , @yaxunl wrote: > The title says `--offline` option, which should be `--offload`. Fixed that and addressed the other comments. The last patch also adds a check to error out if --offload is used in CUDA with

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-20 Thread Daniele Castagna via Phabricator via cfe-commits
dcastagna updated this revision to Diff 401760. dcastagna marked 2 inline comments as done. dcastagna added a comment. Address yaxunl@ comment in Options.td Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117137/new/ https://reviews.llvm.org/D117137

[PATCH] D117137: [Driver] Add CUDA support for --offload param

2022-01-20 Thread Daniele Castagna via Phabricator via cfe-commits
dcastagna updated this revision to Diff 401759. dcastagna marked an inline comment as done. dcastagna retitled this revision from "[Driver] Add CUDA support for --offline param" to "[Driver] Add CUDA support for --offload param". dcastagna added a comment. Address tra@ and yaxunl@ comments. Error