[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan added a comment. In D62574#2183294 , @Anastasia wrote: > In D62574#2136423 , @ebevhan wrote: > >> It seems that D70605 attempted to >> ameliorate the issues that I observed

[PATCH] D62574: Initial draft of target-configurable address spaces.

Anastasia added a comment. > This was also only an initial concept. I think that even once all the issues > with the patch have been ironed out, it would require a round of wider review > since it's a fairly hefty API change. You don't always need a super polished prototype to justify adding

[PATCH] D62574: Initial draft of target-configurable address spaces.

Anastasia added a comment. In D62574#2136423 , @ebevhan wrote: > It seems that D70605 attempted to > ameliorate the issues that I observed (pointer-conversion doing too much), > but it didn't manage to solve the problem

[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan updated this revision to Diff 276163. ebevhan added a comment. Rebased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62574/new/ https://reviews.llvm.org/D62574 Files: clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp clan

[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan added a comment. In D62574#2136553 , @danilaml wrote: > In D62574#2135662 , @ebevhan wrote: > > > It's generally not safe to alter address spaces below the top level. C is > > just very permissive about it.

[PATCH] D62574: Initial draft of target-configurable address spaces.

danilaml added a comment. In D62574#2135662 , @ebevhan wrote: > It's generally not safe to alter address spaces below the top level. C is > just very permissive about it. Isn't that also true for the top-level casts? Unless when it is. And the target s

[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan added a comment. It seems that D70605 attempted to ameliorate the issues that I observed (pointer-conversion doing too much), but it didn't manage to solve the problem fully. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62574/new/ https://rev

[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan added a subscriber: danilaml. ebevhan added a comment. In D62574#2133160 , @danilaml wrote: > What are the remaining roadblocks left before this patch can be merged? I'm > interested in having a target-specific way to define the allowed > explici

[PATCH] D62574: Initial draft of target-configurable address spaces.

danilaml added a comment. What are the remaining roadblocks left before this patch can be merged? I'm interested in having a target-specific way to define the allowed explicit/implicit address space conversions. Also, it appears that currently whether implicit casts between pointers of differen

[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan marked 2 inline comments as done. ebevhan added a comment. Sorry for the very late response on this. Hope it's not completely off your radar. Comment at: include/clang/AST/ASTContext.h:2598 + /// Returns true if address space A overlaps with B. + bool isAddressSpaceO

[PATCH] D62574: Initial draft of target-configurable address spaces.

Anastasia added inline comments. Comment at: include/clang/AST/ASTContext.h:2598 + /// Returns true if address space A overlaps with B. + bool isAddressSpaceOverlapping(LangAS A, LangAS B) const { +// A overlaps with B if either is a superset of the other.

[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan marked 4 inline comments as done. ebevhan added a comment. Sorry for the very late response to this! Comment at: include/clang/AST/ASTContext.h:2598 + /// Returns true if address space A overlaps with B. + bool isAddressSpaceOverlapping(LangAS A, LangAS B) const { +

[PATCH] D62574: Initial draft of target-configurable address spaces.

Anastasia added inline comments. Comment at: include/clang/AST/ASTContext.h:2598 + /// Returns true if address space A overlaps with B. + bool isAddressSpaceOverlapping(LangAS A, LangAS B) const { +// A overlaps with B if either is a superset of the other.

[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan marked 3 inline comments as done. ebevhan added a comment. In D62574#1552220 , @Anastasia wrote: > Ok, I think at some point you might want to test extra functionality that > doesn't fit into OpenCL model, for example explicit conversion over > n

[PATCH] D62574: Initial draft of target-configurable address spaces.

Anastasia added a comment. In D62574#1534159 , @ebevhan wrote: > >> I'll have to see if that's possible without breaking a few more > >> interfaces, since you can throw around Qualifiers and check for > >> compatibility without an ASTContext today. > >>

[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan updated this revision to Diff 203559. ebevhan added a comment. Replaced `compatiblyIncludes` and its dependents with ASTContext accessors instead. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62574/new/ https://reviews.llvm.org/D62574 Files: include/clang/AST/ASTContext.h

[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan added a comment. >> I'll have to see if that's possible without breaking a few more interfaces, >> since you can throw around Qualifiers and check for compatibility without an >> ASTContext today. >> >>> I was just thinking about testing the new logic. Should we add something >>> like

[PATCH] D62574: Initial draft of target-configurable address spaces.

Anastasia added a comment. In D62574#1527126 , @ebevhan wrote: > In D62574#1523835 , @Anastasia wrote: > > > > This patch does not address the issue with the accessors > > > on Qualifiers (isAddressSpaceSupersetOf,

[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan added a comment. In D62574#1523835 , @Anastasia wrote: > > This patch does not address the issue with the accessors > > on Qualifiers (isAddressSpaceSupersetOf, compatiblyIncludes), > > because I don't know how to solve it without breaking a ton

[PATCH] D62574: Initial draft of target-configurable address spaces.

Anastasia added a comment. > This patch does not address the issue with the accessors > on Qualifiers (isAddressSpaceSupersetOf, compatiblyIncludes), > because I don't know how to solve it without breaking a ton of > rather nice encapsulation. Either, every mention of compatiblyIncludes > must

[PATCH] D62574: Initial draft of target-configurable address spaces.

ebevhan created this revision. ebevhan added reviewers: Anastasia, rjmccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is an initial draft of the support for target-configurable address spaces. Original RFC: http://lists.llvm.org/pipermail/cfe-dev/2019-March/0615