[OpenCL] Split PipeType into ReadPipe/WritePipe

[joey]

Summary

Split the PipeType into two derived classes. This allows Sema to diagnose passing read_only to write_only and vice versa.

[Anastasia]

Earlier related discussion: https://reviews.llvm.org/D17821

[bader]

LGTM. Thanks!
A few minor comments regarding outdated comments and style.

include/clang/AST/ASTContext.h

1124

Please, update the comment to specify that this function return pipe type with '__read_only' access qualifier.

lib/AST/ASTContext.cpp

3341

Please, remove this comment. It's a copy of the comment from the header file.

lib/Serialization/ASTReader.cpp

5806–5807

Please, separate case with an empty line.

[joey]

Committed as r287343.

include/clang/AST/ASTContext.h

class ASTContext : public RefCountedBase<ASTContext> {
   llvm::FoldingSet<PackExpansionType> PackExpansionTypes;
   mutable llvm::FoldingSet<ObjCObjectTypeImpl> ObjCObjectTypes;
   mutable llvm::FoldingSet<ObjCObjectPointerType> ObjCObjectPointerTypes;
   mutable llvm::FoldingSet<DependentUnaryTransformType>
     DependentUnaryTransformTypes;
   mutable llvm::FoldingSet<AutoType> AutoTypes;
   mutable llvm::FoldingSet<AtomicType> AtomicTypes;
   llvm::FoldingSet<AttributedType> AttributedTypes;
-  mutable llvm::FoldingSet<PipeType> PipeTypes;
+  mutable llvm::FoldingSet<ReadPipeType> ReadPipeTypes;
+  mutable llvm::FoldingSet<WritePipeType> WritePipeTypes;
 
   mutable llvm::FoldingSet<QualifiedTemplateName> QualifiedTemplateNames;
   mutable llvm::FoldingSet<DependentTemplateName> DependentTemplateNames;
   mutable llvm::FoldingSet<SubstTemplateTemplateParmStorage> 
     SubstTemplateTemplateParms;
   mutable llvm::ContextualFoldingSet<SubstTemplateTemplateParmPackStorage,
                                      ASTContext&> 
     SubstTemplateTemplateParmPacks;
public:
   /// \brief Return the uniqued reference to the type for a block of the
   /// specified type.
   QualType getBlockPointerType(QualType T) const;
 
   /// Gets the struct used to keep track of the descriptor for pointer to
   /// blocks.
   QualType getBlockDescriptorType() const;
 
   /// \brief Return pipe type for the specified type.
-  QualType getPipeType(QualType T) const;
+  QualType getReadPipeType(QualType T) const;
+  QualType getWritePipeType(QualType T) const;
 
   /// Gets the struct used to keep track of the extended descriptor for
   /// pointer to blocks.
   QualType getBlockDescriptorExtendedType() const;
 
   void setcudaConfigureCallDecl(FunctionDecl *FD) {
     cudaConfigureCallDecl = FD;
   }

include/clang/AST/Type.h

   }
   static bool classof(const Type *T) {
     return T->getTypeClass() == Atomic;
   }
 };
 
 /// PipeType - OpenCL20.
 class PipeType : public Type, public llvm::FoldingSetNode {
+protected:
   QualType ElementType;
+  bool isRead;
 
-  PipeType(QualType elemType, QualType CanonicalPtr) :
+  PipeType(QualType elemType, QualType CanonicalPtr, bool isRead) :
     Type(Pipe, CanonicalPtr, elemType->isDependentType(),
          elemType->isInstantiationDependentType(),
          elemType->isVariablyModifiedType(),
          elemType->containsUnexpandedParameterPack()),
-    ElementType(elemType) {}
-  friend class ASTContext;  // ASTContext creates these.
+    ElementType(elemType), isRead(isRead) {}
 
 public:
-
   QualType getElementType() const { return ElementType; }
 
   bool isSugared() const { return false; }
 
   QualType desugar() const { return QualType(this, 0); }
 
   void Profile(llvm::FoldingSetNodeID &ID) {
     Profile(ID, getElementType());
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID, QualType T) {
     ID.AddPointer(T.getAsOpaquePtr());
   }
 
-
   static bool classof(const Type *T) {
     return T->getTypeClass() == Pipe;
   }
 
+  bool isReadOnly() const { return isRead; }
+};
+
+class ReadPipeType : public PipeType {
+  ReadPipeType(QualType elemType, QualType CanonicalPtr) :
+    PipeType(elemType, CanonicalPtr, true) {}
+  friend class ASTContext;  // ASTContext creates these.
+};
+
+class WritePipeType : public PipeType {
+  WritePipeType(QualType elemType, QualType CanonicalPtr) :
+    PipeType(elemType, CanonicalPtr, false) {}
+  friend class ASTContext;  // ASTContext creates these.
 };
 
 /// A qualifier set is used to build a set of qualifiers.
 class QualifierCollector : public Qualifiers {
 public:
   QualifierCollector(Qualifiers Qs = Qualifiers()) : Qualifiers(Qs) {}
 
   /// Collect any qualifiers on the given type and return an

include/clang/Sema/Sema.h

 
   QualType BuildMemberPointerType(QualType T, QualType Class,
                                   SourceLocation Loc,
                                   DeclarationName Entity);
   QualType BuildBlockPointerType(QualType T,
                                  SourceLocation Loc, DeclarationName Entity);
   QualType BuildParenType(QualType T);
   QualType BuildAtomicType(QualType T, SourceLocation Loc);
-  QualType BuildPipeType(QualType T,
+  QualType BuildReadPipeType(QualType T,
+                         SourceLocation Loc);
+  QualType BuildWritePipeType(QualType T,
                          SourceLocation Loc);
 
   TypeSourceInfo *GetTypeForDeclarator(Declarator &D, Scope *S);
   TypeSourceInfo *GetTypeForDeclaratorCast(Declarator &D, QualType FromTy);
   TypeSourceInfo *GetTypeSourceInfoForDeclarator(Declarator &D, QualType T,
                                                TypeSourceInfo *ReturnTypeInfo);
 
   /// \brief Package the given type and TSI into a ParsedType.

include/clang/Serialization/ASTBitCodes.h

enum TypeCode {
       /// \brief A UnaryTransformType record.
       TYPE_UNARY_TRANSFORM       = 39,
       /// \brief An AtomicType record.
       TYPE_ATOMIC                = 40,
       /// \brief A DecayedType record.
       TYPE_DECAYED               = 41,
       /// \brief An AdjustedType record.
       TYPE_ADJUSTED              = 42,
-      /// \brief A PipeType record.
-      TYPE_PIPE                  = 43,
+      /// \brief A ReadPipeType record.
+      TYPE_READ_PIPE             = 43,
       /// \brief An ObjCTypeParamType record.
-      TYPE_OBJC_TYPE_PARAM       = 44
+      TYPE_OBJC_TYPE_PARAM       = 44,
+      /// \brief A WritePipeType record.
+      TYPE_WRITE_PIPE            = 45,
     };
 
     /// \brief The type IDs for special types constructed by semantic
     /// analysis.
     ///
     /// The constants in this enumeration are indices into the
     /// SPECIAL_TYPES record.
     enum SpecialTypeIDs {

lib/AST/ASTContext.cpp

   FunctionProtoType::ExtProtoInfo newEPI = EPI;
   new (FTP) FunctionProtoType(ResultTy, ArgArray, Canonical, newEPI);
   Types.push_back(FTP);
   if (!Unique)
     FunctionProtoTypes.InsertNode(FTP, InsertPos);
   return QualType(FTP, 0);
 }
 
 /// Return pipe type for the specified type.
-QualType ASTContext::getPipeType(QualType T) const {
+QualType ASTContext::getReadPipeType(QualType T) const {
   llvm::FoldingSetNodeID ID;
-  PipeType::Profile(ID, T);
+  ReadPipeType::Profile(ID, T);
 
   void *InsertPos = 0;
-  if (PipeType *PT = PipeTypes.FindNodeOrInsertPos(ID, InsertPos))
+  if (ReadPipeType *PT = ReadPipeTypes.FindNodeOrInsertPos(ID, InsertPos))
     return QualType(PT, 0);
 
   // If the pipe element type isn't canonical, this won't be a canonical type
   // either, so fill in the canonical type field.
   QualType Canonical;
   if (!T.isCanonical()) {
-    Canonical = getPipeType(getCanonicalType(T));
+    Canonical = getReadPipeType(getCanonicalType(T));
 
     // Get the new insert position for the node we care about.
-    PipeType *NewIP = PipeTypes.FindNodeOrInsertPos(ID, InsertPos);
+    ReadPipeType *NewIP = ReadPipeTypes.FindNodeOrInsertPos(ID, InsertPos);
     assert(!NewIP && "Shouldn't be in the map!");
     (void)NewIP;
   }
-  PipeType *New = new (*this, TypeAlignment) PipeType(T, Canonical);
+  ReadPipeType *New = new (*this, TypeAlignment) ReadPipeType(T, Canonical);
   Types.push_back(New);
-  PipeTypes.InsertNode(New, InsertPos);
+  ReadPipeTypes.InsertNode(New, InsertPos);
+  return QualType(New, 0);
+}
+
+QualType ASTContext::getWritePipeType(QualType T) const {
+  llvm::FoldingSetNodeID ID;
+  WritePipeType::Profile(ID, T);
+
+  void *InsertPos = 0;
+  if (WritePipeType *PT = WritePipeTypes.FindNodeOrInsertPos(ID, InsertPos))
+    return QualType(PT, 0);
+
+  // If the pipe element type isn't canonical, this won't be a canonical type
+  // either, so fill in the canonical type field.
+  QualType Canonical;
+  if (!T.isCanonical()) {
+    Canonical = getWritePipeType(getCanonicalType(T));
+
+    // Get the new insert position for the node we care about.
+    WritePipeType *NewIP = WritePipeTypes.FindNodeOrInsertPos(ID, InsertPos);
+    assert(!NewIP && "Shouldn't be in the map!");
+    (void)NewIP;
+  }
+  WritePipeType *New = new (*this, TypeAlignment) WritePipeType(T, Canonical);
+  Types.push_back(New);
+  WritePipeTypes.InsertNode(New, InsertPos);
   return QualType(New, 0);
 }
 
 #ifndef NDEBUG
 static bool NeedsInjectedClassNameType(const RecordDecl *D) {
   if (!isa<CXXRecordDecl>(D)) return false;
   const CXXRecordDecl *RD = cast<CXXRecordDecl>(D);
   if (isa<ClassTemplatePartialSpecializationDecl>(RD))

 /// typesAreCompatible - C99 6.7.3p9: For two qualified types to be compatible,
 /// both shall have the identically qualified version of a compatible type.
 /// C99 6.2.7p1: Two types have compatible types if their types are the
 /// same. See 6.7.[2,3,5] for additional rules.
 bool ASTContext::typesAreCompatible(QualType LHS, QualType RHS,
                                     bool CompareUnqualified) {
   if (getLangOpts().CPlusPlus)
     return hasSameType(LHS, RHS);
-  
   return !mergeTypes(LHS, RHS, false, CompareUnqualified).isNull();
 }
 
 bool ASTContext::propertyTypesAreCompatible(QualType LHS, QualType RHS) {
   return typesAreCompatible(LHS, RHS);
 }
 
 bool ASTContext::typesAreBlockPointerCompatible(QualType LHS, QualType RHS) {

     }
     QualType ResultType = mergeTypes(LHSValue, RHSValue, false,
                                      Unqualified);
     if (ResultType.isNull()) return QualType();
     if (getCanonicalType(LHSValue) == getCanonicalType(ResultType))
       return LHS;
     if (getCanonicalType(RHSValue) == getCanonicalType(ResultType))
       return RHS;
-    return getPipeType(ResultType);
+    return isa<ReadPipeType>(LHS) ? getReadPipeType(ResultType)
+                                  : getWritePipeType(ResultType);
   }
   }
 
   llvm_unreachable("Invalid Type::Class!");
 }
 
 bool ASTContext::doFunctionTypesMatchOnExtParameterInfos(
                    const FunctionProtoType *firstFnType,

lib/AST/TypePrinter.cpp

void TypePrinter::printAtomicBefore(const AtomicType *T, raw_ostream &OS) {
   OS << ')';
   spaceBeforePlaceHolder(OS);
 }
 void TypePrinter::printAtomicAfter(const AtomicType *T, raw_ostream &OS) { }
 
 void TypePrinter::printPipeBefore(const PipeType *T, raw_ostream &OS) {
   IncludeStrongLifetimeRAII Strong(Policy);
 
+  if (T->isReadOnly())
+    OS << "read_only ";
+  else
+    OS << "write_only ";
   OS << "pipe ";
   print(T->getElementType(), OS, StringRef());
   spaceBeforePlaceHolder(OS);
 }
 
 void TypePrinter::printPipeAfter(const PipeType *T, raw_ostream &OS) {
 }
 /// Appends the given scope to the end of a string.

lib/Sema/SemaType.cpp

 /// \brief Build a Pipe type.
 ///
 /// \param T The type to which we'll be building a Pipe.
 ///
 /// \param Loc We do not use it for now.
 ///
 /// \returns A suitable pipe type, if there are no errors. Otherwise, returns a
 /// NULL type.
-QualType Sema::BuildPipeType(QualType T, SourceLocation Loc) {
-  assert(!T->isObjCObjectType() && "Should build ObjCObjectPointerType");
+QualType Sema::BuildReadPipeType(QualType T, SourceLocation Loc) {
+  return Context.getReadPipeType(T);
+}
 
-  // Build the pipe type.
-  return Context.getPipeType(T);
+QualType Sema::BuildWritePipeType(QualType T, SourceLocation Loc) {
+  return Context.getWritePipeType(T);
 }
 
 /// Check whether the specified array size makes the array type a VLA.  If so,
 /// return true, if not, return the size of the array in SizeVal.
 static bool isArraySizeVLA(Sema &S, Expr *ArraySize, llvm::APSInt &SizeVal) {
   // If the size is an ICE, it certainly isn't a VLA. If we're in a GNU mode
   // (like gnu99, but not c99) accept any evaluatable value as an extension.
   class VLADiagnoser : public Sema::VerifyICEDiagnoser {

         D.setInvalidType(true);
       } else if (DeclType.Mem.TypeQuals) {
         T = S.BuildQualifiedType(T, DeclType.Loc, DeclType.Mem.TypeQuals);
       }
       break;
     }
 
     case DeclaratorChunk::Pipe: {
-      T = S.BuildPipeType(T, DeclType.Loc );
+      T = S.BuildReadPipeType(T, DeclType.Loc);
+      processTypeAttrs(state, T, TAL_DeclSpec,
+                       D.getDeclSpec().getAttributes().getList());
       break;
     }
     }
 
     if (T.isNull()) {
       D.setInvalidType(true);
       T = Context.IntTy;
     }

       break;
       #include "clang/Basic/OpenCLImageTypes.def"
     default:
       assert(0 && "Unable to find corresponding image type.");
     }
 
     S.Diag(TypedefTy->getDecl()->getLocStart(),
        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);
+    }
   }
 }
 
 static void processTypeAttrs(TypeProcessingState &state, QualType &type,
                              TypeAttrLocation TAL, AttributeList *attrs) {
   // Scan through and apply attributes to this type where it makes sense.  Some
   // attributes (such as __address_space__, __vector_size__, etc) apply to the
   // type, but others can be present in the type specifiers even though they

lib/Sema/TreeTransform.h

 
   /// \brief Build a new atomic type given its value type.
   ///
   /// By default, performs semantic analysis when building the atomic type.
   /// Subclasses may override this routine to provide different behavior.
   QualType RebuildAtomicType(QualType ValueType, SourceLocation KWLoc);
 
   /// \brief Build a new pipe type given its value type.
-  QualType RebuildPipeType(QualType ValueType, SourceLocation KWLoc);
+  QualType RebuildPipeType(QualType ValueType, SourceLocation KWLoc,
+                           bool isReadPipe);
 
   /// \brief Build a new template name given a nested name specifier, a flag
   /// indicating whether the "template" keyword was provided, and the template
   /// that the template name refers to.
   ///
   /// By default, builds the new template name directly. Subclasses may override
   /// this routine to provide different behavior.
   TemplateName RebuildTemplateName(CXXScopeSpec &SS,

 QualType TreeTransform<Derived>::TransformPipeType(TypeLocBuilder &TLB,
                                                    PipeTypeLoc TL) {
   QualType ValueType = getDerived().TransformType(TLB, TL.getValueLoc());
   if (ValueType.isNull())
     return QualType();
 
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() || ValueType != TL.getValueLoc().getType()) {
-    Result = getDerived().RebuildPipeType(ValueType, TL.getKWLoc());
+    const PipeType *PT = Result->getAs<PipeType>();
+    bool isReadPipe = PT->isReadOnly();
+    Result = getDerived().RebuildPipeType(ValueType, TL.getKWLoc(), isReadPipe);
     if (Result.isNull())
       return QualType();
   }
 
   PipeTypeLoc NewTL = TLB.push<PipeTypeLoc>(Result);
   NewTL.setKWLoc(TL.getKWLoc());
 
   return Result;

 template<typename Derived>
 QualType TreeTransform<Derived>::RebuildAtomicType(QualType ValueType,
                                                    SourceLocation KWLoc) {
   return SemaRef.BuildAtomicType(ValueType, KWLoc);
 }
 
 template<typename Derived>
 QualType TreeTransform<Derived>::RebuildPipeType(QualType ValueType,
-                                                   SourceLocation KWLoc) {
-  return SemaRef.BuildPipeType(ValueType, KWLoc);
+                                                 SourceLocation KWLoc,
+                                                 bool isReadPipe) {
+  return isReadPipe ? SemaRef.BuildReadPipeType(ValueType, KWLoc)
+                    : SemaRef.BuildWritePipeType(ValueType, KWLoc);
 }
 
 template<typename Derived>
 TemplateName
 TreeTransform<Derived>::RebuildTemplateName(CXXScopeSpec &SS,
                                             bool TemplateKW,
                                             TemplateDecl *Template) {
   return SemaRef.Context.getQualifiedTemplateName(SS.getScopeRep(), TemplateKW,

lib/Serialization/ASTReader.cpp

     if (Record.size() != 1) {
       Error("Incorrect encoding of atomic type");
       return QualType();
     }
     QualType ValueType = readType(*Loc.F, Record, Idx);
     return Context.getAtomicType(ValueType);
   }
 
-  case TYPE_PIPE: {
+  case TYPE_READ_PIPE: {
     if (Record.size() != 1) {
       Error("Incorrect encoding of pipe type");
       return QualType();
     }
 
     // Reading the pipe element type.
     QualType ElementType = readType(*Loc.F, Record, Idx);
-    return Context.getPipeType(ElementType);
+    return Context.getReadPipeType(ElementType);
+  }
+  case TYPE_WRITE_PIPE: {
+    if (Record.size() != 1) {
+      Error("Incorrect encoding of pipe type");
+      return QualType();
+    }
+
+    // Reading the pipe element type.
+    QualType ElementType = readType(*Loc.F, Record, Idx);
+    return Context.getWritePipeType(ElementType);
   }
   }
   llvm_unreachable("Invalid TypeCode!");
 }
 
 void ASTReader::readExceptionSpec(ModuleFile &ModuleFile,
                                   SmallVectorImpl<QualType> &Exceptions,
                                   FunctionProtoType::ExceptionSpecInfo &ESI,

lib/Serialization/ASTWriter.cpp

 ASTTypeWriter::VisitAtomicType(const AtomicType *T) {
   Record.AddTypeRef(T->getValueType());
   Code = TYPE_ATOMIC;
 }
 
 void
 ASTTypeWriter::VisitPipeType(const PipeType *T) {
   Record.AddTypeRef(T->getElementType());
-  Code = TYPE_PIPE;
+  if (T->isReadOnly())
+    Code = TYPE_READ_PIPE;
+  else
+    Code = TYPE_WRITE_PIPE;
 }
 
 namespace {
 
 class TypeLocWriter : public TypeLocVisitor<TypeLocWriter> {
   ASTRecordWriter &Record;
 
 public:

test/Misc/ast-dump-pipe.cl

 // RUN: %clang_cc1 -triple spir64 -cl-std=CL2.0 -ast-dump -ast-dump-filter pipetype %s | FileCheck -strict-whitespace %s
 typedef pipe int pipetype;
-// CHECK:      PipeType {{.*}} 'pipe int'
+// CHECK:      PipeType {{.*}} 'read_only pipe int'
+// CHECK-NEXT:   BuiltinType {{.*}} 'int'
+
+typedef read_only pipe int pipetype2;
+// CHECK:      PipeType {{.*}} 'read_only pipe int'
+// CHECK-NEXT:   BuiltinType {{.*}} 'int'
+
+typedef write_only pipe int pipetype3;
+// CHECK:      PipeType {{.*}} 'write_only pipe int'
 // CHECK-NEXT:   BuiltinType {{.*}} 'int'

test/SemaOpenCL/access-qualifier.cl

 
 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}}
 
 #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'}}
+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

test/SemaOpenCL/invalid-pipes-cl2.0.cl

 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
 
 void test1(pipe int *p) {// expected-error {{pipes packet types cannot be of reference type}}
 }
 void test2(pipe p) {// expected-error {{missing actual type specifier for pipe}}
 }
 void test3(int pipe p) {// expected-error {{cannot combine with previous 'int' declaration specifier}}
 }
 void test4() {
-  pipe int p; // expected-error {{type 'pipe int' can only be used as a function parameter}}
+  pipe int p; // expected-error {{type 'read_only pipe int' can only be used as a function parameter}}
   //TODO: fix parsing of this pipe int (*p);
 }
 
 void test5(pipe int p) {
-  p+p; // expected-error{{invalid operands to binary expression ('pipe int' and 'pipe int')}}
-  p=p; // expected-error{{invalid operands to binary expression ('pipe int' and 'pipe int')}}
-  &p; // expected-error{{invalid argument type 'pipe int' to unary expression}}
-  *p; // expected-error{{invalid argument type 'pipe int' to unary expression}}
+  p+p; // expected-error{{invalid operands to binary expression ('read_only pipe int' and 'read_only pipe int')}}
+  p=p; // expected-error{{invalid operands to binary expression ('read_only pipe int' and 'read_only pipe int')}}
+  &p; // expected-error{{invalid argument type 'read_only pipe int' to unary expression}}
+  *p; // expected-error{{invalid argument type 'read_only pipe int' to unary expression}}
 }
 
 typedef pipe int pipe_int_t;
-pipe_int_t test6() {} // expected-error{{declaring function return value of type 'pipe_int_t' (aka 'pipe int') is not allowed}}
+pipe_int_t test6() {} // expected-error{{declaring function return value of type 'pipe_int_t' (aka 'read_only pipe int') is not allowed}}