[PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

Yaxun Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 16 10:45:13 PST 2016

yaxunl marked 3 inline comments as done.
yaxunl added a comment.

In https://reviews.llvm.org/D21698#624967, @Anastasia wrote:

> LGTM! Small nitpicks below can be done before committing. Also it would be nice to double check the compile time is still fine after the last rebase.

Thanks. I did the measurement again and did not see significant difference in compilation time before/after this change.

Comment at: include/clang/Basic/OpenCLOptions.h:43
-  // Enable or disable all options.
-  void setAll(bool Enable = true) {
-#define OPENCLEXT(nm)   nm = Enable;
-#include "clang/Basic/OpenCLExtensions.def"
+  // Is supported OpenCL extension or (optional) core feature for OpenCL version
+  // \p CLVer.
Anastasia wrote:
> Did you mean "and (optional) core feature?"
My understanding is that once an extension becomes an (optional) core feature, it is no longer called an extension. So for for a specific OpenCL version, \p Ext is either is an extension, or an (optional) core feature.  However this wording may be confusing. How about rewording as

Is supported as either an extension or an (optional) core feature for OpenCL version \p CLVer

Comment at: test/SemaOpenCL/extension-begin.cl:5
+// Test with pch.
+// RUN: %clang_cc1 %s -DHEADER -triple spir-unknown-unknown -emit-pch -DHEADER_ONLY -o %t -verify -pedantic
+// RUN: %clang_cc1 %s -DHEADER_USER -triple spir-unknown-unknown -include-pch %t -fsyntax-only -verify -pedantic
Anastasia wrote:
> Do we need -DHEADER_ONLY here?
I will remove it.


More information about the cfe-commits mailing list