[PATCH] D24235: [OpenCL] Improve double literal handling

Sven van Haastregt via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 10 03:11:31 PST 2016


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





More information about the cfe-commits mailing list