[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added a comment. In D54862#1333700 , @ebevhan wrote: > I'm also a bit confused about the semantics that this patch is applying to > function types. It mostly seems to concern the extra trailing Qualifiers on > CXXMethodDecl to store the addrspac

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

ebevhan added a comment. I'm also a bit confused about the semantics that this patch is applying to function types. It mostly seems to concern the extra trailing Qualifiers on CXXMethodDecl to store the addrspace quals, but in some places (SemaType:4842, SemaDecl:3198) it seems to be applying t

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added inline comments. Comment at: lib/CodeGen/CGCall.cpp:77 + if (MD) +RecTy = Context.getAddrSpaceQualType(RecTy, MD->getType().getAddressSpace()); return Context.getPointerType(CanQualType::CreateUnsafe(RecTy)); ebevhan wrote: > I'm a bit lat

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

ebevhan added inline comments. Comment at: lib/CodeGen/CGCall.cpp:77 + if (MD) +RecTy = Context.getAddrSpaceQualType(RecTy, MD->getType().getAddressSpace()); return Context.getPointerType(CanQualType::CreateUnsafe(RecTy)); I'm a bit late to the party, bu

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added a comment. In D54862#1328290 , @mikael wrote: > Seems like my this commit broke: > http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-buildserver/builds/33176 > > But since I don't really know what anything about lldb, I probably wo

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

riccibruno added inline comments. Comment at: include/clang/AST/Type.h:3692 FunctionType::ExceptionType, Expr *, FunctionDecl *, - FunctionType::ExtParameterInfo> { friend class ASTContext; // ASTContext creates these. You can use `Function

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael added a comment. Seems like my this commit broke: http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-buildserver/builds/33176 But since I don't really know what anything about lldb, I probably won't be able to fix it fast enough. /lldb-buildbot/buildServerSlave/lldb-android-b

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

This revision was automatically updated to reflect the committed changes. Closed by commit rC348927: [OpenCL] Add generic AS to 'this' pointer (authored by mikael, committed by ). Changed prior to commit: https://reviews.llvm.org/D54862?vs=177477&id=177846#toc Repository: rC Clang CHANGES S

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

Anastasia accepted this revision. Anastasia added a comment. LGTM! Apart from small improvement that can done before committing if it works at all. :) Comment at: lib/Sema/SemaType.cpp:5169 +TypeSourceInfo *Sema::GetTypeForDeclarator(Declarator &D, Scope *S, +

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael marked an inline comment as done. mikael added inline comments. Comment at: lib/Sema/SemaType.cpp:5169 +TypeSourceInfo *Sema::GetTypeForDeclarator(Declarator &D, Scope *S, + const DeclContext *DC) { // Determine the type of the

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael marked an inline comment as done. mikael added inline comments. Comment at: test/SemaOpenCLCXX/address-space-templates.cl:7 T f1(); // expected-error{{function type may not be qualified with an address space}} - void f2(T); // expected-error{{parameter may not be

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

Anastasia added inline comments. Comment at: lib/Sema/SemaType.cpp:3929 +static TypeSourceInfo * +GetFullTypeForDeclarator(TypeProcessingState &state, QualType declSpecType, + TypeSourceInfo *TInfo, From the `state` you can use `getDeclara

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael marked 4 inline comments as done. mikael added inline comments. Comment at: lib/Sema/SemaDecl.cpp:3196 + +QualType AdjustedQT = QualType(AdjustedType, 0); +LangAS AS = Old->getType().getAddressSpace(); When merging the class function and the file c

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael added a comment. I rebased on Friday, and noticed that I broke two tests: Failing Tests (2): Clang :: CodeGenOpenCLCXX/template-address-spaces.cl Clang :: SemaOpenCLCXX/address-space-templates.cl This upload contains a few extra fixes. CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael updated this revision to Diff 177477. mikael marked an inline comment as not done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54862/new/ https://reviews.llvm.org/D54862 Files: include/clang/AST/CanonicalType.h include/clang/AST/DeclCXX.h include/clang/AST/Type.h include

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

akyrtzi added inline comments. Comment at: lib/Index/USRGeneration.cpp:274 +if (unsigned quals = MD->getTypeQualifiers().getCVRUQualifiers()) Out << (char)('0' + quals); switch (MD->getRefQualifier()) { rjmccall wrote: > akyrtzi wrote: > > rjmccal

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM, but you can probably undo one of my requests. :) Comment at: lib/AST/Type.cpp:2950 + FunctionTypeBits.HasExtQuals = 0; + } } mikael wrote: > mi

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael marked an inline comment as done and an inline comment as not done. mikael added inline comments. Comment at: lib/AST/Type.cpp:2950 + FunctionTypeBits.HasExtQuals = 0; + } } mikael wrote: > rjmccall wrote: > > The indentation here is messed up. > > >

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael updated this revision to Diff 176943. mikael added a comment. - Added FIXME's. - Fixed the latest comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54862/new/ https://reviews.llvm.org/D54862 Files: include/clang/AST/CanonicalType.h include/clang/AST/DeclCXX.h include

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael marked 3 inline comments as done. mikael added inline comments. Comment at: lib/AST/Type.cpp:2950 + FunctionTypeBits.HasExtQuals = 0; + } } rjmccall wrote: > The indentation here is messed up. > > You seem to be mixing up "fast qualifiers" with "CVR q

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added inline comments. Comment at: lib/AST/ItaniumMangle.cpp:1507 +Qualifiers MethodQuals = Qualifiers::fromCVRUMask( +Method->getTypeQualifiers().getCVRUQualifiers()); // We do not consider restrict a distinguishing attribute for overloading ---

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

akyrtzi added inline comments. Comment at: lib/Index/USRGeneration.cpp:274 +if (unsigned quals = MD->getTypeQualifiers().getCVRUQualifiers()) Out << (char)('0' + quals); switch (MD->getRefQualifier()) { rjmccall wrote: > Paging @akyrtzi here. The

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael marked an inline comment as done. mikael added inline comments. Comment at: lib/AST/ItaniumMangle.cpp:1507 +Qualifiers MethodQuals = Qualifiers::fromCVRUMask( +Method->getTypeQualifiers().getCVRUQualifiers()); // We do not consider restrict a distinguishin

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael updated this revision to Diff 176831. mikael added a comment. I uploaded a new patch for most of the comments. I did leave some things out since they need clarification. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54862/new/ https://reviews.llvm.org/D54862 Files: include/cl

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

Anastasia added inline comments. Comment at: lib/Sema/SemaOverload.cpp:1146 +unsigned OldQuals = OldMethod->getTypeQualifiers().getCVRUQualifiers(); +unsigned NewQuals = NewMethod->getTypeQualifiers().getCVRUQualifiers(); if (!getLangOpts().CPlusPlus14 && NewMethod->

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added inline comments. Comment at: lib/AST/ItaniumMangle.cpp:1507 +Qualifiers MethodQuals = Qualifiers::fromCVRUMask( +Method->getTypeQualifiers().getCVRUQualifiers()); // We do not consider restrict a distinguishing attribute for overloading ---

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

Anastasia added inline comments. Comment at: lib/AST/ItaniumMangle.cpp:1507 +Qualifiers MethodQuals = Qualifiers::fromCVRUMask( +Method->getTypeQualifiers().getCVRUQualifiers()); // We do not consider restrict a distinguishing attribute for overloading --

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael marked an inline comment as done and an inline comment as not done. mikael added a comment. Thanks for the feedback, I'll work on fixing the issues! Comment at: lib/Sema/SemaOverload.cpp:1146 +unsigned OldQuals = OldMethod->getTypeQualifiers().getCVRUQualifiers(); +

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added a subscriber: akyrtzi. rjmccall added a comment. I have a lot of comments about various things that need to be fixed, but I just want to take a moment to say that this is a tremendously impressive patch: given only some very high-level directions, you've done a great job figuring

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

Anastasia added inline comments. Comment at: lib/Sema/SemaType.cpp:4816 +// OpenCLCPlusPlus: A class member function has an address space. +if (state.getSema().getLangOpts().OpenCLCPlusPlus && +state.getDeclarator().getContext() == mik

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael marked an inline comment as done. mikael added inline comments. Comment at: lib/Sema/SemaType.cpp:4816 +// OpenCLCPlusPlus: A class member function has an address space. +if (state.getSema().getLangOpts().OpenCLCPlusPlus && +state.getDeclarator(

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

Anastasia added inline comments. Comment at: lib/Sema/SemaType.cpp:4816 +// OpenCLCPlusPlus: A class member function has an address space. +if (state.getSema().getLangOpts().OpenCLCPlusPlus && +state.getDeclarator().getContext() == Can

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael updated this revision to Diff 176596. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54862/new/ https://reviews.llvm.org/D54862 Files: include/clang/AST/CanonicalType.h include/clang/AST/DeclCXX.h include/clang/AST/Type.h lib/AST/ASTContext.cpp lib/AST/ASTDumper.cpp lib

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added inline comments. Comment at: include/clang/AST/DeclCXX.h:2189 +C.addCVRUQualifiers(CVRU); +C.addQualifiers(getType().getQualifiers()); +return C; The address-space qualifier should get stored in the same way on the `FunctionProtoType`

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael marked an inline comment as done. mikael added inline comments. Comment at: lib/Sema/SemaOverload.cpp:5254 + if (!Context.hasSameType(From->getType(), DestType)) { +if (From->getType().getAddressSpace() != DestType.getAddressSpace()) + From = ImpCastExprToType(Fr

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael updated this revision to Diff 176063. Herald added subscribers: arphaman, eraman. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54862/new/ https://reviews.llvm.org/D54862 Files: include/clang/AST/DeclCXX.h include/clang/AST/Type.h lib/AST/DeclCXX.cpp lib/AST/ItaniumMangle.

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added inline comments. Comment at: lib/AST/DeclCXX.cpp:2187 ClassTy = C.getQualifiedType(ClassTy, Qualifiers::fromCVRUMask(getTypeQualifiers())); + I feel like the right design now is for `getTypeQualifiers()` to return

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael updated this revision to Diff 175646. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54862/new/ https://reviews.llvm.org/D54862 Files: lib/AST/DeclCXX.cpp lib/CodeGen/CGCall.cpp lib/CodeGen/CGClass.cpp lib/CodeGen/CGDeclCXX.cpp lib/CodeGen/CGExprCXX.cpp lib/Sema/SemaTyp

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

Anastasia added inline comments. Comment at: lib/CodeGen/CGCall.cpp:80 +// used with the same version of generated operators. +RecTy = Context.getAddrSpaceQualType(RecTy, LangAS::opencl_generic); + rjmccall wrote: > Anastasia wrote: > > rjmccall wrote: >

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added inline comments. Comment at: lib/CodeGen/CGCall.cpp:4035 + V = Builder.CreatePointerBitCastOrAddrSpaceCast(V, DestTy); +} mikael wrote: > rjmccall wrote: > > Always use the `performAddrSpaceConversion` target hook if there's a >

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael marked an inline comment as done. mikael added inline comments. Comment at: lib/CodeGen/CGCall.cpp:4035 + V = Builder.CreatePointerBitCastOrAddrSpaceCast(V, DestTy); +} rjmccall wrote: > Always use the `performAddrSpaceConversion` target

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added inline comments. Comment at: lib/CodeGen/CGCall.cpp:80 +// used with the same version of generated operators. +RecTy = Context.getAddrSpaceQualType(RecTy, LangAS::opencl_generic); + Anastasia wrote: > rjmccall wrote: > > I would suggest tak

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

Anastasia added inline comments. Comment at: lib/CodeGen/CGCall.cpp:80 +// used with the same version of generated operators. +RecTy = Context.getAddrSpaceQualType(RecTy, LangAS::opencl_generic); + rjmccall wrote: > I would suggest taking this opportunity

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added inline comments. Comment at: lib/CodeGen/CGCall.cpp:80 +// used with the same version of generated operators. +RecTy = Context.getAddrSpaceQualType(RecTy, LangAS::opencl_generic); + I would suggest taking this opportunity to set up the AST

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

mikael created this revision. mikael added a reviewer: Anastasia. Herald added subscribers: cfe-commits, yaxunl. Address spaces are cast into generic before invoking the constructor. Repository: rC Clang https://reviews.llvm.org/D54862 Files: lib/AST/DeclCXX.cpp lib/CodeGen/CGCall.cpp