rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land.
In https://reviews.llvm.org/D35056#834705, @rnk wrote: > In https://reviews.llvm.org/D35056#834689, @rsmith wrote: > > > I also removed some incorrect assumptions from the Win64 ABI code; this > > changed the behavior of one testcase from uncopyable-args.cpp > > (`implicitly_deleted_copy_ctor::A` is now passed indirect). > > > That's probably not correct, let me take a look... I remember breaking the > rules for small types here. Nevermind, everything looks good there. Thanks for untangling the mess. I only have comments on comments. ================ Comment at: include/clang/AST/DeclCXX.h:420 + /// \brief True if this class has at least one non-deleted copy or move + /// constructor. That would allow passing it by registers. ---------------- Isn't this "... at least one *trivial*, non-deleted copy or move constructor..."? ================ Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:836 - // If this is true, the implicit copy constructor that Sema would have - // created would not be deleted. FIXME: We should provide a more direct way - // for CodeGen to ask whether the constructor was deleted. - if (!RD->hasUserDeclaredCopyConstructor() && - !RD->hasUserDeclaredMoveConstructor() && - !RD->needsOverloadResolutionForMoveConstructor() && - !RD->hasUserDeclaredMoveAssignment() && - !RD->needsOverloadResolutionForMoveAssignment()) - return RAA_Default; - - // Otherwise, Sema should have created an implicit copy constructor if - // needed. - assert(!RD->needsImplicitCopyConstructor()); - - // We have to make sure the trivial copy constructor isn't deleted. - for (const CXXConstructorDecl *CD : RD->ctors()) { - if (CD->isCopyConstructor()) { - assert(CD->isTrivial()); - // We had at least one undeleted trivial copy ctor. Return directly. - if (!CD->isDeleted()) - return RAA_Default; + // Win64 passes objects with non-deleted, non-trivial copy ctors indirectly. + // ---------------- This doesn't seem to match what we've computing, and it doesn't seem quite right. MSVC will pass a class with deleted, trivial copy ctors indirectly. Would it be correct to rephrase like this? "If RD has at least one trivial, non-deleted copy constructor, it is passed directly. Otherwise, it is passed indirectly." ================ Comment at: test/CodeGenCXX/uncopyable-args.cpp:101 + +// In MSVC 2013, the copy ctor is not deleted by a move assignment. In MSVC 2015, it is. +// WIN64-18-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(i64 ---------------- Oh dear. :( Repository: rL LLVM https://reviews.llvm.org/D35056 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
