Add option -fhip-kernel-arg-name to emit kernel argument
name metadata, which is needed for certain HIP applications.
Details
- Reviewers
tra b-sumner MaskRay - Commits
- rG8ad4c6e4b129: [HIP] add -fhip-kernel-arg-name
Diff Detail
Event Timeline
clang/lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
1845–1846 | Should we consolidate both options into -fkernel-arg-info and make -cl-kernel-arg-info an alias to it? | |
1845–1846 | Also, this check is odd. For some reason only *arg name* metadata is set conditionally, but for whatever reason OpenCL sets other arg metadata unconditionally. Now I'm really curious what's so special about "kernel_arg_name" vs the other arg metadata. |
clang/lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
1845–1846 |
-cl-kernel-arg-info is an OpenCL option defined in OpenCL spec, therefore is made OpenCL only option. It would be confusing to allow it with other languages. | |
1845–1846 |
The other metadata are mandatory because they are necessary for OpenCL runtime to set kernel argument. The kernel argument name is emitted only with -cl-kernel-arg-info since it is only used to support clGetKernelArgInfo which requires -cl-kernel-arg-info to work. |
clang/lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
1845–1846 |
On the other hand having two options that control exactly the same functionality also looks odd to me. The way I see it is that an entity may have more than one name. OpenCL standard requires that that particular functionality must be enabled via -cl-kernel-arg-info and I'm not proposing to change that. The OpenCL standard has no say whether that flag may have a different name in addition to the standard-required one. This is similar to how over time we've been transitioning what used to be CUDA-only options into their generic -gpu and -offload variants, only in this case OpenCL functionality becomes useful outside of OpenCL. |
clang/lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
1845–1846 |
If we introduce a generic option -fkernel-arg-info and make -cl-kernel-arg-info alias to it, do we want to make it available to all languages or limit it to HIP and OpenCL only? |
clang/lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
1845–1846 |
I think that would be fine. |
clang/lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
1845–1846 |
ShouldParseIf appears to support expressions, so restricting it to hip + OCL may be as simple as ShouldParseIf<!strconcat(open_cl.KeyPath, "||", hip.KeyPath)>; I'm not sure what to do about -cl-kernel-arg-info being in Group<opencl_Group>. AFAICT, option currently may be only in one group. It's not a problem for now as only OpenCL arg needs to be in a group, but that is a point against merging the options. That makes my suggestion a wash -- a bit better here, possibly worse there. I withdraw the suggestion. |
Should we consolidate both options into -fkernel-arg-info and make -cl-kernel-arg-info an alias to it?