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

[Yaxun Liu via Phabricator via cfe-commits]

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.


https://reviews.llvm.org/D21698