[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

2021-09-15 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. https://reviews.llvm.org/D109841 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103835/new/ https://reviews.llvm.org/D103835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

2021-09-15 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. I see. Yes, in that case, that sounds right. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103835/new/ https://reviews.llvm.org/D103835 ___ cfe-commits mailing list cfe-commits

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

2021-09-15 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. In D103835#3001011 , @rjmccall wrote: > Hmm. I think "v-tables are in the address space of the object pointer" is > not a good assumption. Probably this ought to be determined by the C++ ABI > for the target. In principle it

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

2021-09-14 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Hmm. I think "v-tables are in the address space of the object pointer" is not a good assumption. Probably this ought to be determined by the C++ ABI for the target. In principle it could even be class-specific, but I think we can start by assuming it's universal.

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

2021-09-14 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added a comment. In D103835#2999731 , @yaxunl wrote: > In D103835#2999545 , @arichardson > wrote: > >> Merging this change broke our out-of-tree CHERI targets (and Arm Morello). >> We use addrspace(2

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

2021-09-14 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. In D103835#2999545 , @arichardson wrote: > Merging this change broke our out-of-tree CHERI targets (and Arm Morello). We > use addrspace(200) for *all* globals (including vtables). It would have been > nice if I had been added t

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

2021-09-14 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added a comment. Merging this change broke our out-of-tree CHERI targets (and Arm Morello). We use addrspace(200) for *all* globals (including vtables). It would have been nice if I had been added to this review considering that I added line you are changing here. If vtables are no

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

2021-06-08 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG054cc3b1b469: [CUDA][HIP] Fix store of vtbl in ctor (authored by yaxunl). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103835

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

2021-06-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision. yaxunl added reviewers: tra, rjmccall. yaxunl requested review of this revision. vtbl itself is in default global address space. When clang emits ctor, it gets a pointer to the vtbl field based on the `this` pointer, then stores vtbl to the pointer. Since `this` poin