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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to