[OpenCL] -cl-ext option can overwrite OpenCL features imported from a module

[AlexeySotkin]

Summary

When we compile a PCH cl_khr_fp64 is enabled by default.
And its support is imported alongside with the PCH.
With this patch we can disable features enabled in the PCH we import

[AlexeySotkin]

Added tests

[bader]

test/SemaOpenCL/extensions-import.cl

14

You can use #ifdef cl_khr_fp64 to check if extension is supported.

[Anastasia]

include/clang/Basic/OpenCLOptions.h

132

May be the name could be setAll since it's doing both enable and disable too.

test/SemaOpenCL/extensions-import.cl

6

Do we need to test w/o PCH here? I think we have similar test coverage in test/SemaOpenCL/extensions.cl.

Also we normally put RUN lines first.

include/clang/Basic/OpenCLOptions.h

public:
   bool isSupportedCore(llvm::StringRef Ext, unsigned CLVer) const {
     auto I = OptMap.find(Ext)->getValue();
     return I.Supported && I.Avail <= CLVer &&
       I.Core != ~0U && CLVer >= I.Core;
   }
 
   // Is supported OpenCL extension for OpenCL version \p CLVer.
   // For supported (optional) core feature, return false.
   bool isSupportedExtension(llvm::StringRef Ext, unsigned CLVer) const {
     auto I = OptMap.find(Ext)->getValue();
     return I.Supported && I.Avail <= CLVer &&
       (I.Core == ~0U || CLVer < I.Core);
   }
 
+  llvm::StringRef split(llvm::StringRef Ext, bool& V) {
+    switch (Ext[0]) {
+    case '+':
+      V = true;
+      Ext = Ext.drop_front();
+      break;
+    case '-':
+      V = false;
+      Ext = Ext.drop_front();
+      break;
+    }
+    return Ext;
+  }
+
   void enable(llvm::StringRef Ext, bool V = true) {
+    assert(!Ext.empty() && "Extension is empty.");
+
+    Ext = split(Ext, V);
+
+    if (Ext.equals("all")) {
+      enableAll(V);
+      return;
+    }
     OptMap[Ext].Enabled = V;
   }
 
   /// \brief Enable or disable support for OpenCL extensions
   /// \param Ext name of the extension optionally prefixed with
   ///        '+' or '-'
   /// \param V used when \p Ext is not prefixed by '+' or '-'
   void support(llvm::StringRef Ext, bool V = true) {
     assert(!Ext.empty() && "Extension is empty.");
 
-    switch (Ext[0]) {
-    case '+':
-      V = true;
-      Ext = Ext.drop_front();
-      break;
-    case '-':
-      V = false;
-      Ext = Ext.drop_front();
-      break;
-    }
+    Ext = split(Ext, V);
 
     if (Ext.equals("all")) {
       supportAll(V);
       return;
     }
     OptMap[Ext].Supported = V;
   }
 
#include "clang/Basic/OpenCLExtensions.def"
 
   // Turn on or off support of all options.
   void supportAll(bool On = true) {
     for (llvm::StringMap<Info>::iterator I = OptMap.begin(),
          E = OptMap.end(); I != E; ++I)
       I->second.Supported = On;
   }
 
-  void disableAll() {
+  void enableAll(bool On = true) {
     for (llvm::StringMap<Info>::iterator I = OptMap.begin(),
          E = OptMap.end(); I != E; ++I)
-      I->second.Enabled = false;
+      I->second.Enabled = On;
+  }
+
+  void disableAll() {
+    enableAll(false);
   }
 
   void enableSupportedCore(unsigned CLVer) {
     for (llvm::StringMap<Info>::iterator I = OptMap.begin(),
          E = OptMap.end(); I != E; ++I)
       if (isSupportedCore(I->getKey(), CLVer))
         I->second.Enabled = true;
   }
 
   friend class ASTWriter;
   friend class ASTReader;
 };
 
 }  // end namespace clang
 
 #endif

lib/Serialization/ASTReader.cpp

if (!FPPragmaOptions.empty()) {
     assert(FPPragmaOptions.size() == 1 && "Wrong number of FP_PRAGMA_OPTIONS");
     SemaObj->FPFeatures = FPOptions(FPPragmaOptions[0]);
   }
 
   SemaObj->OpenCLFeatures.copy(OpenCLExtensions);
   SemaObj->OpenCLTypeExtMap = OpenCLTypeExtMap;
   SemaObj->OpenCLDeclExtMap = OpenCLDeclExtMap;
 
+  // OpenCL features imported from a module can be overwritten by -cl-ext option
+  for (const auto &Ext :
+       getContext().getTargetInfo().getTargetOpts().OpenCLExtensionsAsWritten) {
+    SemaObj->OpenCLFeatures.support(Ext);
+    SemaObj->OpenCLFeatures.enable(Ext);
+  }
+
   UpdateSema();
 }
 
 void ASTReader::UpdateSema() {
   assert(SemaObj && "no Sema to update");
 
   // Load the offsets of the declarations that Sema references.
   // They will be lazily deserialized when needed.

test/SemaOpenCL/extensions-import.h

+extern double foo();

test/SemaOpenCL/extensions-import.cl

+// When we compile a PCH cl_khr_fp64 is enabled by default.
+// And its support is imported alongside with the PCH.
+// Check that we can disable features enabled in the PCH we import.
+
+// No PCH
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL1.2 %s -fsyntax-only -verify -DFP64
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL1.2 %s -fsyntax-only -verify -cl-ext=-cl_khr_fp64
+
+// With PCH
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL1.2 -I %S -emit-pch -o %t.pch %S/extensions-import.h
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL1.2 -include-pch %t.pch %s -fsyntax-only -verify -DIMPORT -DFP64
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL1.2 -include-pch %t.pch %s -fsyntax-only -verify -DIMPORT -cl-ext=-cl_khr_fp64
+
+#ifdef FP64
+// expected-no-diagnostics
+#else
+// expected-error@+4 {{type 'double' requires cl_khr_fp64 extension}}
+// expected-error@+4 {{type 'double' requires cl_khr_fp64 extension}}
+// expected-warning@+4 {{double precision constant requires cl_khr_fp64}}
+#endif
+void kernel f1(double da) {
+  double d;
+  (void) 1.0;
+#ifdef IMPORT
+  d = foo();
+#endif
+}