Simplifying the OpenCL image attribute. It does not need a semantic integer parameter because the required information is encoded in the spelling. Added an appropriate subject to the attribute, and simplified the semantic checking (which will likely be expanded upon in a future patch). Also, removed the GNU spelling since it was unsupported in the first place.

llvm-svn: 199229

clang/include/clang/Basic/Attr.td

@@ -453,16 +453,19 @@ def OpenCLKernel : InheritableAttr {
   let Subjects = SubjectList<[Function], ErrorDiag>;
 }
 
+// This attribute is both a type attribute, and a declaration attribute (for
+// parameter variables).
 def OpenCLImageAccess : Attr {
-  let Spellings = [GNU<"opencl_image_access">,
-                   Keyword<"__read_only">, Keyword<"read_only">,
+  let Spellings = [Keyword<"__read_only">, Keyword<"read_only">,
                    Keyword<"__write_only">, Keyword<"write_only">,
                    Keyword<"__read_write">, Keyword<"read_write">];
-
-  // The access argument is used by the GNU syntax when parsing the attribute,
-  // but is used by the semantic attribute for all syntaxes (the keywords imply
-  // a specific access value).
-  let Args = [IntArgument<"Access">];
+  let Subjects = SubjectList<[ParmVar], ErrorDiag>;
+  let Accessors = [Accessor<"isReadOnly", [Keyword<"__read_only">,
+                                           Keyword<"read_only">]>,
+                   Accessor<"isReadWrite", [Keyword<"__read_write">,
+                                            Keyword<"read_write">]>,
+                   Accessor<"isWriteOnly", [Keyword<"__write_only">,
+                                            Keyword<"write_only">]>];
 }
 
 def OpenCLPrivateAddressSpace : TypeAttr {

clang/include/clang/Basic/OpenCL.h

@@ -1,29 +0,0 @@
-//===--- OpenCL.h - OpenCL enums --------------------------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-///
-/// \file
-/// \brief Defines some OpenCL-specific enums.
-///
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_CLANG_BASIC_OPENCL_H
-#define LLVM_CLANG_BASIC_OPENCL_H
-
-namespace clang {
-
-/// \brief Names for the OpenCL image access qualifiers (OpenCL 1.1 6.6).
-enum OpenCLImageAccess {
-  CLIA_read_only = 1,
-  CLIA_write_only = 2,
-  CLIA_read_write = 3
-};
-
-}
-
-#endif

clang/lib/CodeGen/CodeGenFunction.cpp

@@ -22,7 +22,6 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/StmtCXX.h"
-#include "clang/Basic/OpenCL.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/Frontend/CodeGenOptions.h"
@@ -412,10 +411,11 @@ static void GenOpenCLArgMetadata(const FunctionDecl *FD, llvm::Function *Fn,
     // Get image access qualifier:
     if (ty->isImageType()) {
       const OpenCLImageAccessAttr *A = parm->getAttr<OpenCLImageAccessAttr>();
-      if (A && A->getAccess() == CLIA_write_only)
+      if (A && A->isWriteOnly())
         accessQuals.push_back(llvm::MDString::get(Context, "write_only"));
       else
         accessQuals.push_back(llvm::MDString::get(Context, "read_only"));
+      // FIXME: what about read_write?
     } else
       accessQuals.push_back(llvm::MDString::get(Context, "none"));
 

clang/lib/Parse/ParseDecl.cpp

@@ -16,7 +16,6 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/CharInfo.h"
-#include "clang/Basic/OpenCL.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ParsedTemplate.h"
@@ -654,31 +653,8 @@ void Parser::ParseOpenCLAttributes(ParsedAttributes &attrs) {
 void Parser::ParseOpenCLQualifiers(ParsedAttributes &Attrs) {
   IdentifierInfo *AttrName = Tok.getIdentifierInfo();
   SourceLocation AttrNameLoc = Tok.getLocation();
-  ArgsUnion Expr;
-  switch (Tok.getKind()) {
-    // OpenCL qualifiers:
-    case tok::kw___private:
-    case tok::kw___global:
-    case tok::kw___local:
-    case tok::kw___constant:
-      // These are handled automatically below and have no args.
-      break;
-    case tok::kw___read_only:
-      Expr = Actions.ActOnIntegerConstant(SourceLocation(),
-                                          CLIA_read_only).take();
-      break;
-    case tok::kw___write_only:
-      Expr = Actions.ActOnIntegerConstant(SourceLocation(),
-                                          CLIA_write_only).take();
-      break;
-    case tok::kw___read_write:
-      Expr = Actions.ActOnIntegerConstant(SourceLocation(),
-                                          CLIA_read_write).take();
-      break;
-    default: llvm_unreachable("Unknown OpenCL qualifier");
-  }
-  Attrs.addNew(AttrName, AttrNameLoc, 0, AttrNameLoc, &Expr,
-               Expr.isNull() ? 0 : 1, AttributeList::AS_Keyword);
+  Attrs.addNew(AttrName, AttrNameLoc, 0, AttrNameLoc, 0, 0,
+               AttributeList::AS_Keyword);
 }
 
 /// \brief Parse a version number.

clang/lib/Sema/SemaDeclAttr.cpp

@@ -3182,12 +3182,9 @@ static void handleCallConvAttr(Sema &S, Decl *D, const AttributeList &Attr) {
 
 static void handleOpenCLImageAccessAttr(Sema &S, Decl *D,
                                         const AttributeList &Attr) {
-  uint32_t ArgNum;
-  if (!checkUInt32Argument(S, Attr, Attr.getArgAsExpr(0), ArgNum))
-    return;
-
-  D->addAttr(::new (S.Context) OpenCLImageAccessAttr(Attr.getRange(),
-                                                     S.Context, ArgNum));
+  D->addAttr(::new (S.Context)
+              OpenCLImageAccessAttr(Attr.getRange(), S.Context,
+                                    Attr.getAttributeSpellingListIndex()));
 }
 
 bool Sema::CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC, 
@@ -4176,8 +4173,7 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
   case AttributeList::AT_OpenCLKernel:
     handleSimpleAttribute<OpenCLKernelAttr>(S, D, Attr); break;
   case AttributeList::AT_OpenCLImageAccess:
-    handleOpenCLImageAccessAttr(S, D, Attr);
-    break;
+    handleSimpleAttribute<OpenCLImageAccessAttr>(S, D, Attr); break;
 
   // Microsoft attributes:
   case AttributeList::AT_MsStruct:

clang/lib/Sema/SemaType.cpp

@@ -22,7 +22,6 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeLocVisitor.h"
-#include "clang/Basic/OpenCL.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Preprocessor.h"
@@ -4613,43 +4612,6 @@ void Sema::adjustMemberFunctionCC(QualType &T, bool IsStatic) {
   T = Context.getAdjustedType(T, Wrapped);
 }
 
-/// Handle OpenCL image access qualifiers: read_only, write_only, read_write
-static void HandleOpenCLImageAccessAttribute(QualType& CurType,
-                                             const AttributeList &Attr,
-                                             Sema &S) {
-  // Check the attribute arguments.
-  if (Attr.getNumArgs() != 1) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments)
-      << Attr.getName() << 1;
-    Attr.setInvalid();
-    return;
-  }
-  Expr *sizeExpr = static_cast<Expr *>(Attr.getArgAsExpr(0));
-  llvm::APSInt arg(32);
-  if (sizeExpr->isTypeDependent() || sizeExpr->isValueDependent() ||
-      !sizeExpr->isIntegerConstantExpr(arg, S.Context)) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_argument_type)
-      << Attr.getName() << AANT_ArgumentIntegerConstant
-      << sizeExpr->getSourceRange();
-    Attr.setInvalid();
-    return;
-  }
-  unsigned iarg = static_cast<unsigned>(arg.getZExtValue());
-  switch (iarg) {
-  case CLIA_read_only:
-  case CLIA_write_only:
-  case CLIA_read_write:
-    // Implemented in a separate patch
-    break;
-  default:
-    // Implemented in a separate patch
-    S.Diag(Attr.getLoc(), diag::err_attribute_invalid_size)
-      << sizeExpr->getSourceRange();
-    Attr.setInvalid();
-    break;
-  }
-}
-
 /// HandleVectorSizeAttribute - this attribute is only applicable to integral
 /// and float scalars, although arrays, pointers, and function return values are
 /// allowed in conjunction with this construct. Aggregates with this attribute
@@ -4941,7 +4903,8 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
       attr.setUsedAsTypeAttr();
       break;
     case AttributeList::AT_OpenCLImageAccess:
-      HandleOpenCLImageAccessAttribute(type, attr, state.getSema());
+      // FIXME: there should be some type checking happening here, I would
+      // imagine, but the original handler's checking was entirely superfluous.
       attr.setUsedAsTypeAttr();
       break;
 

clang/test/SemaOpenCL/invalid-kernel-attrs.cl

@@ -28,5 +28,6 @@ constant int foo3 __attribute__((vec_type_hint(char))) = 0; // expected-error {{
 
 void f_kernel_image2d_t( kernel image2d_t image ) { // expected-error {{'kernel' attribute only applies to functions}}
   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}}
 }