yaxunl added inline comments.

Comment at: lib/Sema/SemaDecl.cpp:7599-7602
@@ -7595,3 +7598,6 @@
     // of event_t type.
-    S.Diag(Param->getLocation(), diag::err_bad_kernel_param_type) << PT;
+    // Do not diagnose half type since it is diagnosed as invalid argument
+    // type for any function eleswhere.
+    if (!PT->isHalfType())
+      S.Diag(Param->getLocation(), diag::err_bad_kernel_param_type) << PT;
bader wrote:
> yaxunl wrote:
> > bader wrote:
> > > It looks strange to me. First we check if parameter type is half - we set 
> > > InvalidKernelParam status, later we check again and do not report an 
> > > error.
> > > I think it would be simpler just return ValidKernelParam for half data 
> > > type from getOpenCLKernelParameterType,
> > > 
> > > I think the whole patch should two deleted lines from 
> > > getOpenCLKernelParameterType function + test case.
> > getOpenCLKernelParameterType should report half type as InvalidKernelParam 
> > since it really is an invalid kernel argument type. We do not emit 
> > diagnostic msg because the msg is redundant, not because half type is a 
> > valid kernel argument type. 
> > 
> > getOpenCLKernelParameterType may be used for other purpose. Reporting half 
> > type as a valid kernel argument violates the semantics of 
> > getOpenCLKernelParameterType and can cause confusion and potential error.
> Maybe we should the other way.
> Leave half parameter check here only and remove duplicate check that reports 
> "declaring function parameter of type 'half' is not allowed; did you forget * 
> ?" message.
Clang can emit two error msgs in this case since it does two independent checks:

1. half type cannot be used as function argument when cl_khr_fp16 is disabled

2. half type cannot be used as kernel function argument when cl_khr_fp16 is 

I think error msg 1 is better than error msg 2 since error msg 1 provides more 
information, i.e., when cl_khr_fp16 is disabled, half type cannot be used as 
either kernel function argument or non-kernel function argument, whereas error 
msg 2 may give user the wrong impression that half type can not be used for 
kernel function argument only.


cfe-commits mailing list

Reply via email to