tra added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1845-1846
+ }
+ if (getCodeGenOpts().EmitOpenCLArgMetadata ||
+ getCodeGenOpts().HIPSaveKernelArgName)
Fn->setMetadata("kernel_arg_name",
----------------
yaxunl wrote:
> yaxunl wrote:
> > tra wrote:
> > > tra wrote:
> > > > Should we consolidate both options into `-fkernel-arg-info` and make
> > > > `-cl-kernel-arg-info` an alias to it?
> > > 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.
> > > Should we consolidate both options into `-fkernel-arg-info` and make
> > > `-cl-kernel-arg-info` an alias to it?
> >
> > -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.
> > 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.
>
> 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.
> It would be confusing to allow it with other languages.
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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128022/new/
https://reviews.llvm.org/D128022
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
