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

Reply via email to