[PATCH] D16040: [OpenCL] Refine OpenCLImageAccessAttr to OpenCLAccessAttr

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4934
@@ +4933,3 @@
+    const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
+    if (AccessAttr->isReadWrite()) {
+      if (DeclTy->isPipeType() ||
----------------
In the case of failure, where would destructor of AccessAttr be called?

Could we invoke it before exiting? Or alternatively it's possible to check an access kind based on string matching from Attr getName() and create AccessAttr later on success.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:5694
@@ +5693,3 @@
+    // attribute and may cause the same attribute to be added twice
+    if (const ParmVarDecl *PDecl = llvm::dyn_cast<ParmVarDecl>(D)) {
+      if (PDecl->getType().getCanonicalType().getTypePtr()->isPipeType())
----------------
So where would the attribute check happen? I don't think the comment is very clear.

================
Comment at: lib/Sema/SemaType.cpp:6223
@@ +6222,3 @@
+                                   Sema &S) {
+  // OpenCL v2.0 s6.6 -- Access Qualifier can used only for image and pipe type
+  if (!(CurType->isImageType() || CurType->isPipeType())) {
----------------
remove one -

================
Comment at: test/SemaOpenCL/invalid-kernel-attrs.cl:31
@@ -30,3 +30,3 @@
   int __kernel x; // expected-error {{'__kernel' attribute only applies to functions}}
-  read_only int i; // expected-error {{'read_only' attribute only applies to parameters}}
-  __write_only int j; // expected-error {{'__write_only' attribute only applies to parameters}}
+  read_only image1d_t i; // expected-error {{'read_only' attribute only applies to parameters}}
+  __write_only image2d_t j; // expected-error {{'__write_only' attribute only applies to parameters}}
----------------
Strangely, I don't see this diagnostic being added in this patch.


http://reviews.llvm.org/D16040