[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

rjmccall added a comment. In D70172#1812664 , @yaxunl wrote: > In D70172#1812631 , @rjmccall wrote: > > > Most uses of the destructor do not use the delete operator, though, and > > therefore should not trigger the

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

yaxunl added a comment. In D70172#1812631 , @rjmccall wrote: > Most uses of the destructor do not use the delete operator, though, and > therefore should not trigger the diagnostics in `f` to be emitted. And this > really doesn't require a fully-realize

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

yaxunl added a comment. In D70172#1810665 , @rsmith wrote: > This doesn't look quite right to me. I don't think we should treat the > `delete this;` for a destructor as being emitted-for-device in any > translation unit in which the vtable is marked used

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

rjmccall added a comment. In D70172#1812533 , @yaxunl wrote: > In D70172#1809571 , @rjmccall wrote: > > > I thought you were saying that the destructor decl hadn't been created yet, > > but I see now that you're sa

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

yaxunl added a comment. In D70172#1809571 , @rjmccall wrote: > I thought you were saying that the destructor decl hadn't been created yet, > but I see now that you're saying something more subtle. > > `CurContext` is set to the destructor because the stan

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

yaxunl updated this revision to Diff 237122. yaxunl added a comment. Add tests for device compilation. Add a test when both vtbl and deleting dtor are emitted with diagnostic due to delete operator. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70172/new/ https://reviews.llvm.org/D701

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

rsmith added a comment. Herald added a subscriber: herhut. This doesn't look quite right to me. I don't think we should treat the `delete this;` for a destructor as being emitted-for-device in any translation unit in which the vtable is marked used. (For example, if in your testcase `MSEmitDele

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

rjmccall added a comment. I thought you were saying that the destructor decl hadn't been created yet, but I see now that you're saying something more subtle. `CurContext` is set to the destructor because the standard says in [class.dtor]p13: At the point of definition of a virtual destructor

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

yaxunl added a comment. In D70172#1772140 , @rjmccall wrote: > Richard is definitely our main expert in the implicit synthesis of special > members. It seems to me that if we need the destructor declaration at some > point, we should be forcing it to ex

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

rjmccall added a comment. Richard is definitely our main expert in the implicit synthesis of special members. It seems to me that if we need the destructor declaration at some point, we should be forcing it to exist at that point. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70172/ne

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

rnk added a comment. In D70172#1746451 , @yaxunl wrote: > We are not using Itanium ABI when we do host compilation of CUDA/HIP on > windows. During the host compilation on windows only MS C++ ABI is used. > > This issue is not due to mixing MS ABI with It

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

yaxunl added inline comments. Comment at: clang/test/SemaCUDA/deleting-dtor.cu:45-46 + +// Only MS has diagnostic since MS requires deleting dtor is emitted when +// vtable is emitted, even though dtor is not defined. +namespace MSEmitDeletingDtor { tra wrote: >

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

yaxunl updated this revision to Diff 232197. yaxunl marked 2 inline comments as done. yaxunl added a comment. remove unnecessary states added to Sema. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70172/new/ https://reviews.llvm.org/D70172 Files: clang/lib/Sema/SemaDecl.cpp clang/t

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

yaxunl added a comment. In D70172#1745998 , @rnk wrote: > Are we sure using both Itanium and MS C++ ABIs at the same time is really the > best way forward here? What are the constraints on CUDA that require the > Itanium ABI? I'm sure there are real reas

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

rnk added a comment. Are we sure using both Itanium and MS C++ ABIs at the same time is really the best way forward here? What are the constraints on CUDA that require the Itanium ABI? I'm sure there are real reasons you can't just use the MS ABI as is, but I'm curious what they are. Was there

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

yaxunl added a comment. sorry I think I misunderstood the meaning of "blocking" so I put it back. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70172/new/ https://reviews.llvm.org/D70172 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

rjmccall added a reviewer: rsmith. rjmccall added a subscriber: rsmith. rjmccall added a comment. This seems like the wrong approach; @rsmith should take a look. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70172/new/ https://reviews.llvm.org/D70172 _

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

tra added a subscriber: rnk. tra added a comment. Calling @rnk for Windows know-how. Comment at: clang/test/SemaCUDA/deleting-dtor.cu:45-46 + +// Only MS has diagnostic since MS requires deleting dtor is emitted when +// vtable is emitted, even though dtor is not defined. +name

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

yaxunl created this revision. yaxunl added reviewers: tra, rjmccall. The Microsoft ABI requires that clang performs the destructor body checks (i.e. operator delete() lookup) when the vtable is marked used, as the deleting destructor is emitted with the vtable, not with the destructor definition a