[OpenCL] Relax diagnostics on OpenCL access qualifiers

[AlexeySachkov]

Summary

Emit warning for multiple access qualifiers if they do not conflict.

Patch by Alexey Bader

[Anastasia]

LGTM! Thanks!

test/SemaOpenCL/access-qualifier.cl

107 ↗

(On Diff #162670)

Could we change one write_only to __write_only to increase test coverage.

[AlexeySachkov]

Applied comment from Anastasia

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

 }
 
 static void handleOpenCLAccessAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (D->isInvalidDecl())
     return;
 
   // Check if there is only one access qualifier.
   if (D->hasAttr<OpenCLAccessAttr>()) {
+    if (D->getAttr<OpenCLAccessAttr>()->getSemanticSpelling() ==
+        AL.getSemanticSpelling()) {
+      S.Diag(AL.getLoc(), diag::warn_duplicate_declspec)
+          << AL.getName()->getName() << AL.getRange();
+    } else {
       S.Diag(AL.getLoc(), diag::err_opencl_multiple_access_qualifiers)
           << D->getSourceRange();
       D->setInvalidDecl(true);
       return;
     }
+  }
 
   // OpenCL v2.0 s6.6 - read_write can be used for image types to specify that an
   // image object can be read and written.
   // OpenCL v2.0 s6.13.6 - A kernel cannot read from and write to the same pipe
   // object. Using the read_write (or __read_write) qualifier with the pipe
   // qualifier is a compilation error.
   if (const auto *PDecl = dyn_cast<ParmVarDecl>(D)) {
     const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();

cfe/trunk/lib/Sema/SemaType.cpp

static void HandleOpenCLAccessAttr(QualType &CurType, const ParsedAttr &Attr,
   // OpenCL v2.0 s6.6 - Access qualifier can be used only for image and pipe type.
   if (!(CurType->isImageType() || CurType->isPipeType())) {
     S.Diag(Attr.getLoc(), diag::err_opencl_invalid_access_qualifier);
     Attr.setInvalid();
     return;
   }
 
   if (const TypedefType* TypedefTy = CurType->getAs<TypedefType>()) {
-    QualType PointeeTy = TypedefTy->desugar();
-    S.Diag(Attr.getLoc(), diag::err_opencl_multiple_access_qualifiers);
+    QualType BaseTy = TypedefTy->desugar();
 
     std::string PrevAccessQual;
-    switch (cast<BuiltinType>(PointeeTy.getTypePtr())->getKind()) {
+    if (BaseTy->isPipeType()) {
+      if (TypedefTy->getDecl()->hasAttr<OpenCLAccessAttr>()) {
+        OpenCLAccessAttr *Attr =
+            TypedefTy->getDecl()->getAttr<OpenCLAccessAttr>();
+        PrevAccessQual = Attr->getSpelling();
+      } else {
+        PrevAccessQual = "read_only";
+      }
+    } else if (const BuiltinType* ImgType = BaseTy->getAs<BuiltinType>()) {
+
+      switch (ImgType->getKind()) {
         #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
       case BuiltinType::Id:                                          \
         PrevAccessQual = #Access;                                    \
         break;
         #include "clang/Basic/OpenCLImageTypes.def"
       default:
-      assert(0 && "Unable to find corresponding image type.");
+        llvm_unreachable("Unable to find corresponding image type.");
+      }
+    } else {
+      llvm_unreachable("unexpected type");
+    }
+    StringRef AttrName = Attr.getName()->getName();
+    if (PrevAccessQual == AttrName.ltrim("_")) {
+      // Duplicated qualifiers
+      S.Diag(Attr.getLoc(), diag::warn_duplicate_declspec)
+         << AttrName << Attr.getRange();
+    } else {
+      // Contradicting qualifiers
+      S.Diag(Attr.getLoc(), diag::err_opencl_multiple_access_qualifiers);
     }
 
     S.Diag(TypedefTy->getDecl()->getBeginLoc(),
-           diag::note_opencl_typedef_access_qualifier)
-        << PrevAccessQual;
+           diag::note_opencl_typedef_access_qualifier) << PrevAccessQual;
   } else if (CurType->isPipeType()) {
     if (Attr.getSemanticSpelling() == OpenCLAccessAttr::Keyword_write_only) {
       QualType ElemType = CurType->getAs<PipeType>()->getElementType();
       CurType = S.Context.getWritePipeType(ElemType);
     }
   }
 }
 

cfe/trunk/test/SemaOpenCL/access-qualifier.cl

 kernel void k8(write_only img1d_ro_default img){} // expected-error {{multiple access qualifiers}}
 
 kernel void k9(read_only int i){} // expected-error{{access qualifier can only be used for pipe and image type}}
 
 kernel void k10(read_only Int img){} // expected-error {{access qualifier can only be used for pipe and image type}}
 
 kernel void k11(read_only write_only image1d_t i){} // expected-error{{multiple access qualifiers}}
 
-kernel void k12(read_only read_only image1d_t i){} // expected-error{{multiple access qualifiers}}
+kernel void k12(read_only read_only image1d_t i){} // expected-warning {{duplicate 'read_only' declaration specifier}}
 
 #if __OPENCL_C_VERSION__ >= 200
 kernel void k13(read_write pipe int i){} // expected-error{{access qualifier 'read_write' can not be used for 'read_only pipe int'}}
 #else
 kernel void k13(__read_write image1d_t i){} // expected-error{{access qualifier '__read_write' can not be used for '__read_write image1d_t' prior to OpenCL version 2.0}}
 #endif
 
 #if __OPENCL_C_VERSION__ >= 200
 void myPipeWrite(write_only pipe int); // expected-note {{passing argument to parameter here}}
 kernel void k14(read_only pipe int p) {
   myPipeWrite(p); // expected-error {{passing 'read_only pipe int' to parameter of incompatible type 'write_only pipe int'}}
 }
 #endif
 
 #if __OPENCL_C_VERSION__ < 200
 kernel void test_image3d_wo(write_only image3d_t img) {} // expected-error {{use of type '__write_only image3d_t' requires cl_khr_3d_image_writes extension to be enabled}}
 #endif
+
+#if __OPENCL_C_VERSION__ >= 200
+kernel void read_write_twice_typedef(read_write img1d_rw i){} // expected-warning {{duplicate 'read_write' declaration specifier}}
+// expected-note@-74 {{previously declared 'read_write' here}}
+
+kernel void pipe_ro_twice(read_only read_only pipe int i){} // expected-warning{{duplicate 'read_only' declaration specifier}}
+// Conflicting access qualifiers
+kernel void pipe_ro_twice_tw(read_write read_only read_only pipe int i){} // expected-error{{access qualifier 'read_write' can not be used for 'read_only pipe int'}}
+kernel void pipe_ro_wo(read_only write_only pipe int i){} // expected-error{{multiple access qualifiers}}
+
+typedef read_only pipe int ROPipeInt;
+kernel void pipe_ro_twice_typedef(read_only ROPipeInt i){} // expected-warning{{duplicate 'read_only' declaration specifier}}
+// expected-note@-2 {{previously declared 'read_only' here}}
+
+kernel void pass_ro_typedef_to_wo(ROPipeInt p) {
+  myPipeWrite(p); // expected-error {{passing 'ROPipeInt' (aka 'read_only pipe int') to parameter of incompatible type 'write_only pipe int'}}
+  // expected-note@-25 {{passing argument to parameter here}}
+}
+#endif
+
+kernel void read_only_twice_typedef(__read_only img1d_ro i){} // expected-warning {{duplicate '__read_only' declaration specifier}}
+// expected-note@-95 {{previously declared 'read_only' here}}
+
+kernel void read_only_twice_default(read_only img1d_ro_default img){} // expected-warning {{duplicate 'read_only' declaration specifier}}
+// expected-note@-101 {{previously declared 'read_only' here}}
+
+kernel void image_wo_twice(write_only __write_only image1d_t i){} // expected-warning {{duplicate '__write_only' declaration specifier}}
+kernel void image_wo_twice_typedef(write_only img1d_wo i){} // expected-warning {{duplicate 'write_only' declaration specifier}}
+// expected-note@-103 {{previously declared 'write_only' here}}
+