[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG29a5c3c8fe30: [NVPTX] Introduce attribute to mark kernels without a language mode (authored by jhuber6). Changed prior to commit:

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 updated this revision to Diff 508170. jhuber6 added a comment. Updating to simply add an entirely new attribute again. The existing `CUDAGlobal` attribute does what we want, but it's also highly coupled with the CUDA language. This made it pretty much impossible to find a way to re-use it

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 added a comment. @tra would it be possible to go to the earlier version that simply duplicated a slight amount of logic to introduce the new and separate attribute `nvptx_kernel`? Overloading CUDA's `device` attribute is problematic because it's used and checked in several different

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 added a comment. In D140226#4052105 , @aaron.ballman wrote: > Precommit CI found failures that look relevant to the patch. > >> We may want to be able to mark certain regions as kernels even without being >> in an accepted CUDA or OpenCL

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

aaron.ballman added a reviewer: erichkeane. aaron.ballman added a comment. Precommit CI found failures that look relevant to the patch. > We may want to be able to mark certain regions as kernels even without being > in an accepted CUDA or OpenCL language mode. Can you explain this a bit more?

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1198 -def CUDAGlobal : InheritableAttr { - let Spellings = [GNU<"global">, Declspec<"__global__">]; +def CUDAGlobal : InheritableAttr, TargetSpecificAttr { + let Spellings = [GNU<"global">,

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

shangwuyao added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1198 -def CUDAGlobal : InheritableAttr { - let Spellings = [GNU<"global">, Declspec<"__global__">]; +def CUDAGlobal : InheritableAttr, TargetSpecificAttr { + let Spellings = [GNU<"global">,

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

JonChesterfield added a comment. Calling convention is the right model here. Kernels are functions with a different calling convention to the 'normal' functions in a very literal sense. The calling convention modelling in clang is different to attribute handling and changing nvptx to it is

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 added a comment. In D140226#4003826 , @tschuett wrote: > But then you are maybe mixing two concepts. `kernel` is source code or AST > feature. nvptx or AMDGPU are command line flags. > > CUDA, Sycl, nvtx, and AMDGPU are modes or calling

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

tschuett added a comment. In D140226#4003794 , @jhuber6 wrote: > In D140226#4003788 , @tschuett > wrote: > >> There are already SYCL specific attributes: https://reviews.llvm.org/D60455 > > We could potentially

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 added a comment. In D140226#4003788 , @tschuett wrote: > There are already SYCL specific attributes: https://reviews.llvm.org/D60455 We could potentially merge these all into some generic attribute since they all do the same thing on a

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

tschuett added a comment. There are already SYCL specific attributes: https://reviews.llvm.org/D60455 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140226/new/ https://reviews.llvm.org/D140226 ___

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 added a comment. In D140226#4003781 , @keryell wrote: > I wonder whether we could not factorize some code/attribute/logic with AMDGPU > or SYCL. > Is the use case to have for example CUDA+HIP+SYCL in the same TU and thus > there is a need for

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

keryell added subscribers: bader, keryell. keryell added a comment. I wonder whether we could not factorize some code/attribute/logic with AMDGPU or SYCL. Is the use case to have for example CUDA+HIP+SYCL in the same TU and thus there is a need for different attributes Repository: rG LLVM

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1198 -def CUDAGlobal : InheritableAttr { - let Spellings = [GNU<"global">, Declspec<"__global__">]; +def CUDAGlobal : InheritableAttr, TargetSpecificAttr { + let Spellings = [GNU<"global">,

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

tra added a comment. LGTM. General question -- what happens now that the `global` and `launch_bounds` are target-specific as opposed to language-specific, if they happen to be used in a C++ compilation targeting `x86`? I assume they will still be ignored, right? Comment at:

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 updated this revision to Diff 483640. jhuber6 added a comment. Changing to use the same CUDA global attributes. This requires a few extra checks for whether or not we were in CUDA mode since previously it just assume any time we saw one of these globals we were in that mode. I added a

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

tra added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:7362 + if (FD->hasAttr()) { +addNVVMMetadata(F, "kernel", 1); + } jhuber6 wrote: > tra wrote: > > How does AMDGPU track kernels? It may be a good opportunity to stop using > >

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:7362 + if (FD->hasAttr()) { +addNVVMMetadata(F, "kernel", 1); + } tra wrote: > How does AMDGPU track kernels? It may be a good opportunity to stop using > metadata for this if we

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

tra added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:7362 + if (FD->hasAttr()) { +addNVVMMetadata(F, "kernel", 1); + } How does AMDGPU track kernels? It may be a good opportunity to stop using metadata for this if we can use a better

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 created this revision. jhuber6 added reviewers: tra, JonChesterfield. Herald added subscribers: kosarev, mattd, gchakrabarti, asavonic, jdoerfert, Anastasia, tpr. Herald added a reviewer: aaron.ballman. Herald added a project: All. jhuber6 requested review of this revision. Herald added a