[OpenCL] Minor cleanup to access attributes on images

[joey]

Summary

Use the semantic spelling (an enum) rather than a string, to determine what access qualifier is used.

[aaron.ballman]

lib/Sema/SemaType.cpp

1211–1212

Since we're updating this, can you change attrs to Attrs, and maybe make it a const AttributeList *?

1224

The caller can no longer tell the difference between a real-only OpenCL access attribute and no OpenCL access attribute. I know that this was the effective behavior of the original code, but that's specific to the current use case, so I think this change is a tiny regression in the semantics.

1629

Just checking, but, has this situation already been diagnosed elsewhere?

[joey]

Fixed a latent infinite loop bug in 'getImageAccess', it was dereferencing Attrs, instead of Next.

[joey]

lib/Sema/SemaType.cpp

1224

I renamed this function.

1629

Maybe I should actually replace this with an llvm_unreachable?

[aaron.ballman]

LGTM, with one minor nit.

lib/Sema/SemaType.cpp

1629

I would remove the default label entirely; the switch is now fully covered, so if we remove the label, we can get a diagnostic if a new spelling is added and we forget to update this switch.

[joey]

Committed as r287100.

test/SemaOpenCL/access-qualifier.cl

71

This was an unintentional test change, and was not committed.

lib/Sema/SemaType.cpp

TypeResult Sema::actOnObjCTypeArgsAndProtocolQualifiers(
   if (ObjCObjectTL.getType() == T)
     ObjCObjectTL.getBaseLoc().initializeFullCopy(BaseTypeInfo->getTypeLoc());
   else
     ObjCObjectTL.getBaseLoc().initialize(Context, Loc);
 
   // We're done. Return the completed type to the parser.
   return CreateParsedType(Result, ResultTInfo);
 }
 
-static StringRef getImageAccessAttrStr(AttributeList *attrs) {
-  if (attrs) {
-
-    AttributeList *Next;
+static OpenCLAccessAttr::Spelling getImageAccess(const AttributeList *Attrs) {
+  if (Attrs) {
+    const AttributeList *Next = Attrs;
     do {
-      AttributeList &Attr = *attrs;
+      const AttributeList &Attr = *Next;
       Next = Attr.getNext();
       if (Attr.getKind() == AttributeList::AT_OpenCLAccess) {
-        return Attr.getName()->getName();
+        return static_cast<OpenCLAccessAttr::Spelling>(
+            Attr.getSemanticSpelling());
       }
     } while (Next);
   }
-  return "";
+  return OpenCLAccessAttr::Keyword_read_only;
 }
 
 /// \brief Convert the specified declspec to the appropriate type
 /// object.
 /// \param state Specifies the declarator containing the declaration specifier
 /// to be converted, along with other associated processing state.
 /// \returns The type described by the declaration specifiers.  This function
 /// never returns null.
case DeclSpec::TST_atomic:
     if (Result.isNull()) {
       Result = Context.IntTy;
       declarator.setInvalidType(true);
     }
     break;
 
 #define GENERIC_IMAGE_TYPE(ImgType, Id) \
   case DeclSpec::TST_##ImgType##_t: \
-    Result = llvm::StringSwitch<QualType>( \
-                 getImageAccessAttrStr(DS.getAttributes().getList())) \
-                 .Cases("write_only", "__write_only", Context.Id##WOTy) \
-                 .Cases("read_write", "__read_write", Context.Id##RWTy) \
-                 .Default(Context.Id##ROTy); \
+    switch (getImageAccess(DS.getAttributes().getList())) { \
+    case OpenCLAccessAttr::Keyword_write_only: \
+      Result = Context.Id##WOTy; break; \
+    case OpenCLAccessAttr::Keyword_read_write: \
+      Result = Context.Id##RWTy; break; \
+    case OpenCLAccessAttr::Keyword_read_only: \
+      Result = Context.Id##ROTy; break; \
+    default: assert(0 && "Unknown access attribute!"); \
+    } \
     break;
 #include "clang/Basic/OpenCLImageTypes.def"
 
   case DeclSpec::TST_error:
     Result = Context.IntTy;
     declarator.setInvalidType(true);
     break;
   }

test/SemaOpenCL/access-qualifier.cl

 
 kernel void k12(read_only read_only image1d_t i){} // expected-error{{multiple access qualifiers}}
 
 #if __OPENCL_C_VERSION__ >= 200
 kernel void k13(read_write pipe int i){} // expected-error{{access qualifier 'read_write' can not be used for '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

test/SemaOpenCL/invalid-image.cl

 // RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify -D=ATTR_TEST -fms-compatibility %s
 
 void test1(image1d_t *i) {} // expected-error{{pointer to type '__read_only image1d_t' is invalid in OpenCL}}
 
 void test2(image1d_t i) {
   image1d_t ti;            // expected-error{{type '__read_only image1d_t' can only be used as a function parameter}}
   image1d_t ai[] = {i, i}; // expected-error{{array of '__read_only image1d_t' type is invalid in OpenCL}}
   i=i; // expected-error{{invalid operands to binary expression ('__read_only image1d_t' and '__read_only image1d_t')}}
   i+1; // expected-error{{invalid operands to binary expression ('__read_only image1d_t' and 'int')}}
   &i; // expected-error{{invalid argument type '__read_only image1d_t' to unary expression}}
   *i; // expected-error{{invalid argument type '__read_only image1d_t' to unary expression}}
 }
 
 image1d_t test3() {} // expected-error{{declaring function return value of type '__read_only image1d_t' is not allowed}}
+
+#ifdef ATTR_TEST
+// Test case for an infinite loop bug.
+kernel void foob(read_only __ptr32  image2d_t i) { } // expected-error{{'__ptr32' attribute only applies to pointer arguments}}
+#endif