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:
> 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
disabled
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.
https://reviews.llvm.org/D24666
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
