svenvh added inline comments.
================
Comment at: lib/Sema/SemaExpr.cpp:3431
+ .getSupportedOpenCLOpts()
+ .cl_khr_fp64) ||
getOpenCLOptions().cl_khr_fp64)) {
----------------
yaxunl wrote:
> neil.hickey wrote:
> > yaxunl wrote:
> > > This check
> > > (getLangOpts().OpenCLVersion >= 120 &&
> > > Context.getTargetInfo()
> > > .getSupportedOpenCLOpts()
> > > .cl_khr_fp64)
> > >
> > > is redundant since for CL 1.2 and above getOpenCLOptions().cl_khr_fp64 is
> > > set to be true by default.
> > This is get**Supported**OpenCLOpts(). Some hardware may not support doubles
> In Sema::Initialize(), there is code to initialize enabled extensions:
>
> ```
> // Initialize predefined OpenCL types and supported optional core features.
> if (getLangOpts().OpenCL) {
> #define OPENCLEXT(Ext) \
> if
> (Context.getTargetInfo().getSupportedOpenCLOpts().is_##Ext##_supported_core( \
> getLangOpts().OpenCLVersion)) \
> getOpenCLOptions().Ext = 1;
> #include "clang/Basic/OpenCLExtensions.def"
>
> ```
> `is_##Ext##_supported_core` accounts for two factors: 1. whether the target
> supports the extension; 2. whether the extension has become OpenCL core
> featrue.
>
> Since getOpenCLOptions().cl_khr_fp64 is initialized with the same value as in
> the mentioned check, the check is redundant.
Your new patch no longer does version checks, so there is no need to mention it
in the comments anymore I'd say.
https://reviews.llvm.org/D24235
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
