yaxunl marked 2 inline comments as done. yaxunl added inline comments.
================ Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1845-1846 + } + if (getCodeGenOpts().EmitOpenCLArgMetadata || + getCodeGenOpts().HIPSaveKernelArgName) Fn->setMetadata("kernel_arg_name", ---------------- 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. ================ Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1845-1846 + } + if (getCodeGenOpts().EmitOpenCLArgMetadata || + getCodeGenOpts().HIPSaveKernelArgName) Fn->setMetadata("kernel_arg_name", ---------------- 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. 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