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;
     D.setInvalidType();
----------------
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.


https://reviews.llvm.org/D24666



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to