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

[Xiuli PAN via cfe-commits]

pxli168 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() ||
----------------
Anastasia wrote:
> 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.
Nice catch, I think I will try if getName() can handle this, or I think maybe I need a destructor.


================
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())
----------------
Anastasia wrote:
> So where would the attribute check happen? I don't think the comment is very clear.
Maybe I don't make my self claer.

The pipe decl like
```
SomeFn(read_only pipe int p)
```
It will process read_only twice which is not expected, this will make attr add twice.

I will make the comment clearer to understand.


================
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}}
----------------
Anastasia wrote:
> Strangely, I don't see this diagnostic being added in this patch.
As you can see, the read_only used to use with int. Now we check the type, so int could not used for this test more. Just change the in to image1d_t to keep the old test case work.


http://reviews.llvm.org/D16040