[PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.

================
Comment at: include/clang/Basic/OpenCLOptions.h:39
@@ +38,3 @@
+
+  void set(llvm::StringRef Ext, bool Enable = true) {
+    assert(!Ext.empty() && "Extension is empty.");
----------------
yaxunl wrote:
> Better add a comments for this function about its semantics, i.e., if Ext does not starts with +/-, it is enabled/disabled by \p Enable, otherwise +/- overrides \p Enable, since this is not obvious.
Indeed, generally it would be nice to add a comment explaining the purpose of this functions. I don't think the name is descriptive enough.

================
Comment at: include/clang/Driver/Options.td:394
@@ -393,1 +393,3 @@
   HelpText<"OpenCL only. Specify that single precision floating-point divide and sqrt used in the program source are correctly rounded.">;
+def cl_ext_EQ : CommaJoined<["-"], "cl-ext=">, Group<opencl_Group>, Flags<[CC1Option]>,
+  HelpText<"OpenCL only. Enable or disable specific OpenCL extensions separated by comma. Use 'all' for all extensions.">;
----------------
I would see it as cc1 option instead to avoid confusions on its intension.

================
Comment at: include/clang/Driver/Options.td:395
@@ -394,1 +394,3 @@
+def cl_ext_EQ : CommaJoined<["-"], "cl-ext=">, Group<opencl_Group>, Flags<[CC1Option]>,
+  HelpText<"OpenCL only. Enable or disable specific OpenCL extensions separated by comma. Use 'all' for all extensions.">;
 def client__name : JoinedOrSeparate<["-"], "client_name">;
----------------
Could we also add a short statement, that +/- are used to turn the extesions on and off. 

================
Comment at: lib/Basic/Targets.cpp:1882
@@ -1881,1 +1881,3 @@
+
+    setOpenCLExtensionOpts();
   }
----------------
Is this really target specific? I feel this should rather go into common code.


https://reviews.llvm.org/D23712