[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia added a comment. In D55850#1369008 , @ebevhan wrote: > I think the code that comes to mind is mostly like in > `GetTypeSourceInfoForDeclarator`: > > LangAS AS = > (ASIdx == LangAS::Default ? LangAS::opencl_generic : ASIdx); > > >

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

ebevhan added inline comments. Comment at: lib/Sema/SemaOverload.cpp:5151 + if (FromTypeCanon.getQualifiers().hasAddressSpace()) { +Qualifiers QualsImplicitParamType = ImplicitParamType.getQualifiers(); I tested this patch with some kludges to let it

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

ebevhan added a comment. I think the code that comes to mind is mostly like in `GetTypeSourceInfoForDeclarator`: LangAS AS = (ASIdx == LangAS::Default ? LangAS::opencl_generic : ASIdx); It's behind an OpenCLCPlusPlus guard so it won't add generic on anything if it's not OpenCL++, but

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

rjmccall added a comment. In D55850#1368767 , @Anastasia wrote: > In D55850#1366709 , @rjmccall wrote: > > > Most of the remaining OpenCL checks are for OpenCL-specific logic like > > inferring the default address

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia added a comment. In D55850#1366709 , @rjmccall wrote: > Most of the remaining OpenCL checks are for OpenCL-specific logic like > inferring the default address space, and yeah, we could probably make that a > target option or something. We

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

rjmccall added a comment. I'll just add that we've been trying to not put things behind OpenCL guards as much as possible. Most of the remaining OpenCL checks are for OpenCL-specific logic like inferring the default address space, and yeah, we could probably make that a target option or

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia added a comment. In D55850#1366094 , @ebevhan wrote: > Okay, sounds good! I'm not a C++ expert, but I'd be happy to look at the > patches when they're up. Haven't done much serious testing on my end so far, > but from what I've seen the

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

ebevhan added a comment. Okay, sounds good! I'm not a C++ expert, but I'd be happy to look at the patches when they're up. Haven't done much serious testing on my end so far, but from what I've seen the patches so far look good! Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia added a comment. In D55850#1365437 , @ebevhan wrote: > I'm a bit late to the party here, it was an older patch so I wasn't watching > this one. > > I get a bit miffed when address space related features get locked behind > langoptions that

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

ebevhan added a comment. I'm a bit late to the party here, it was an older patch so I wasn't watching this one. I get a bit miffed when address space related features get locked behind langoptions that aren't strictly address spaces. I know that there are currently a couple code snippets

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia added a comment. After rebase I had to modify the following test: Index: test/SemaOpenCLCXX/address_space_overloading.cl === --- test/SemaOpenCLCXX/address_space_overloading.cl (revision 351746) +++

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

This revision was automatically updated to reflect the committed changes. Closed by commit rC351747: [OpenCL] Allow address spaces as method qualifiers. (authored by stulova, committed by ). Herald added a subscriber: ebevhan. Changed prior to commit:

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Okay, LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55850/new/ https://reviews.llvm.org/D55850 ___ cfe-commits mailing list

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia updated this revision to Diff 182496. Anastasia added a comment. Added a comment explaining when multiple address spaces are diagnosed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55850/new/ https://reviews.llvm.org/D55850 Files: include/clang/AST/Type.h

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

rjmccall added inline comments. Comment at: lib/Parse/ParseDecl.cpp:6170 + } +} + } Anastasia wrote: > rjmccall wrote: > > Does this not need to diagnose redundant qualifiers? Why is this path > > required in addition to the path in

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia updated this revision to Diff 182282. Anastasia added a comment. - Fixed typo in FIXME CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55850/new/ https://reviews.llvm.org/D55850 Files: include/clang/AST/Type.h include/clang/Sema/ParsedAttr.h lib/Parse/ParseDecl.cpp

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Parse/ParseDecl.cpp:6170 + } +} + } rjmccall wrote: > Does this not need to diagnose redundant qualifiers? Why is this path > required in

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia updated this revision to Diff 182281. Anastasia marked an inline comment as done. Anastasia added a comment. - Removed else case in DiagnoseMultipleAddrSpaceAttributes - Added extra test to check correctness of addr space for 'this' when statically used in a class scope. CHANGES

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

rjmccall added inline comments. Comment at: lib/Parse/ParseDecl.cpp:6170 + } +} + } Does this not need to diagnose redundant qualifiers? Why is this path required in addition to the path in SemaType, anyway? Comment at:

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia updated this revision to Diff 182012. Anastasia added a comment. Fixed wording on the comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55850/new/ https://reviews.llvm.org/D55850 Files: include/clang/Sema/ParsedAttr.h lib/Parse/ParseDecl.cpp

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia marked an inline comment as done. Anastasia added a comment. In D55850#1358812 , @rjmccall wrote: > Okay, so is this ready to re-review independently? Yes, please. It should be fine to review on its own. Thanks! CHANGES SINCE LAST ACTION

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

rjmccall added a comment. Okay, so is this ready to re-review independently? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55850/new/ https://reviews.llvm.org/D55850 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia marked 3 inline comments as done. Anastasia added inline comments. Comment at: lib/Sema/SemaOverload.cpp:9279 +(CandAS1 != LangAS::opencl_generic && CandAS1 != LangAS::Default)) + return true; + } rjmccall wrote: > Anastasia wrote: > >

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia updated this revision to Diff 181825. Anastasia added a comment. Rebased on top of recent related changes and addressed remaining review comments. This now depends on: https://reviews.llvm.org/D56735 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55850/new/

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

brunodf added inline comments. Comment at: lib/Sema/SemaType.cpp:4877 T = Context.getFunctionType(T, ParamTys, EPI); T = state.getSema().Context.getAddrSpaceQualType(T, AS); } else { I follow all of the above (from the point "a

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Sema/SemaOverload.cpp:2828 // FIXME: OpenCL: Need to consider address spaces unsigned FromQuals = FromFunction->getTypeQuals().getCVRUQualifiers(); rjmccall

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

rjmccall added inline comments. Comment at: lib/Sema/SemaOverload.cpp:9279 +(CandAS1 != LangAS::opencl_generic && CandAS1 != LangAS::Default)) + return true; + } Anastasia wrote: > rjmccall wrote: > > rjmccall wrote: > > > This at least needs a

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Sema/SemaOverload.cpp:9279 +(CandAS1 != LangAS::opencl_generic && CandAS1 != LangAS::Default)) + return true; + } rjmccall wrote: > rjmccall wrote: > >

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

rjmccall added inline comments. Comment at: lib/Parse/ParseDecl.cpp:6162 +} + } + Anastasia wrote: > rjmccall wrote: > > Anastasia wrote: > > > rjmccall wrote: > > > > This is enforcing a restriction that users write `const __private`, > > > >

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Parse/ParseDecl.cpp:6162 +} + } + rjmccall wrote: > Anastasia wrote: > > rjmccall wrote: > > > This is enforcing a restriction that users write `const

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

rjmccall added inline comments. Comment at: lib/Parse/ParseDecl.cpp:6162 +} + } + Anastasia wrote: > rjmccall wrote: > > This is enforcing a restriction that users write `const __private`, which > > seems unreasonable. It looks like

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia added a comment. In D55850#1335826 , @rjmccall wrote: > You're gating on OpenCL C++ in several places in this patch, but I don't > think you need to. This extension should be available to anyone using > address spaces and C++. Ok, I am

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia added inline comments. Comment at: lib/Parse/ParseDecl.cpp:6162 +} + } + rjmccall wrote: > This is enforcing a restriction that users write `const __private`, which > seems unreasonable. It looks like `ParseTypeQualifierList` takes a

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia updated this revision to Diff 178904. Anastasia marked an inline comment as done. Anastasia added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55850/new/ https://reviews.llvm.org/D55850 Files: lib/Parse/ParseDecl.cpp

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

rjmccall added a comment. You're gating on OpenCL C++ in several places in this patch, but I don't think you need to. This extension should be available to anyone using address spaces and C++. Comment at: lib/Parse/ParseDecl.cpp:6162 +} + } +

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Sema/SemaOverload.cpp:2828 // FIXME: OpenCL: Need to consider address spaces unsigned FromQuals = FromFunction->getTypeQuals().getCVRUQualifiers(); I am still

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia updated this revision to Diff 178767. Anastasia added a comment. Removed FIXME that has been addressed CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55850/new/ https://reviews.llvm.org/D55850 Files: lib/Parse/ParseDecl.cpp lib/Sema/SemaOverload.cpp

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

Anastasia created this revision. Anastasia added a reviewer: rjmccall. Herald added a subscriber: yaxunl. This patch allows qualifying methods with address spaces and extends some overloading rules to use those qualifiers. The main use case is to prevent conversions to generic/default address