rsmith closed this revision.
rsmith added a comment.
Committed as r310401.
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
rnk added inline comments.
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
rsmith wrote
rsmith added inline 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.
rnk wrote:
> Isn't this "... at least one *tri
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
> > c
v.g.vassilev added a comment.
I do not feel qualified enough to review this patch but I added few minor
comments.
Comment at: include/clang/AST/DeclCXX.h:827
+return data().DefaultedCopyConstructorIsDeleted;
+ }
+ /// \brief \c true if a defaulted move constructor for th
v.g.vassilev added a comment.
And thanks for working on this!!
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
rsmith added a comment.
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
> > (`implic
rnk added a comment.
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 probabl
rsmith updated this revision to Diff 110115.
rsmith edited the summary of this revision.
rsmith added a comment.
Herald added a subscriber: klimek.
Remove added calls to `DeclareImplicit*` and `ShouldDeleteSpecialMember`. In
their place, figure out whether an implicit special member would be del
rsmith added a comment.
As requested by Vassil, I'm going to upload another version of this that avoids
declaring implicit special members so frequently.
https://reviews.llvm.org/D35056
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http:/
rjmccall added inline comments.
Comment at: lib/Sema/SemaDeclCXX.cpp:5742
+// effect of performing a trivial copy of the type.
+bool Sema::CanPassInRegisters(CXXRecordDecl *D) {
+ if (D->isDependentType())
This should probably be called something like "computeCa
v.g.vassilev updated this revision to Diff 109932.
v.g.vassilev marked an inline comment as done.
v.g.vassilev added a comment.
We set the record's property denoting whether we can pass the decl by registers
as a last step of `Sema::CheckCompletedCXXClass`. We cannot do it any earlier
than that
rjmccall added inline comments.
Comment at: lib/CodeGen/CGCXXABI.cpp:43
if (RD->hasNonTrivialDestructor())
return false;
v.g.vassilev wrote:
> rjmccall wrote:
> > Does canPassInRegisters() not subsume all of these earlier checks?
> No, if I remove them
v.g.vassilev added inline comments.
Comment at: lib/CodeGen/CGCXXABI.cpp:43
if (RD->hasNonTrivialDestructor())
return false;
rjmccall wrote:
> Does canPassInRegisters() not subsume all of these earlier checks?
No, if I remove them here I get a lot of te
rjmccall added inline comments.
Comment at: lib/CodeGen/CGCXXABI.cpp:43
if (RD->hasNonTrivialDestructor())
return false;
Does canPassInRegisters() not subsume all of these earlier checks?
https://reviews.llvm.org/D35056
v.g.vassilev updated this revision to Diff 108556.
v.g.vassilev added a comment.
Move back the triviality checks in CGCXXABI. Explain why.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGe
v.g.vassilev updated this revision to Diff 108535.
v.g.vassilev added a comment.
Put back accidentally removed test case.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.c
v.g.vassilev added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:14841-14852
+ // If a move constructor or move assignment operator was declared, the
+ // default copy constructors are implicitly deleted, except in one case
+ // related to compatibility with MSVC pre-2015
v.g.vassilev updated this revision to Diff 108533.
v.g.vassilev marked 6 inline comments as done.
v.g.vassilev added a comment.
Address some of the comments.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXA
rsmith added inline comments.
Comment at: lib/CodeGen/CGCXXABI.cpp:33-47
+ // See also Sema::ShouldDeleteSpecialMember. These two functions
+ // should be kept consistent.
+
// If RD has a non-trivial move or copy constructor, we cannot copy the
// argument.
if (RD->ha
v.g.vassilev updated this revision to Diff 108505.
v.g.vassilev added a comment.
Move the checks in Sema.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Sema/Se
rsmith added inline comments.
Comment at: lib/AST/DeclCXX.cpp:1476
+
+ data().HasNonDeletedCopyOrMoveConstructor = !CopyOrMoveDeleted;
}
This is wrong. "Has a non-deleted copy or move constructor" is not the same
thing as "does not have a deleted copy or move
rjmccall added a comment.
Thank you, I like this approach much better, and the IRGen changes seem fine to
me. I'd like to defer to someone else (probably Richard) to review whether the
changes to completeDefinition() are correct; I'm not up-to-date with how we
handle lazy declarations.
https
v.g.vassilev updated this revision to Diff 105776.
v.g.vassilev added a comment.
Remove accidentally added change in SemaDecl.cpp
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/Itanium
v.g.vassilev updated this revision to Diff 105775.
v.g.vassilev added a comment.
Compute only once if there is a non-deleted move or copy constructor and store
them in the `DefinitionData` of the `CXXRecordDecl`.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/AS
rnk added a comment.
I discussed this with @rsmith and we think the correct fix is to update the
DefinitionData bits computed when adding special members. I haven't reviewed
this patch in detail, but we came to the conclusion that Clang's optimization
to avoid implicit special member declaratio
rjmccall added a comment.
If this is a common algorithm across all ABIs, can we just put Sema / the AST
in charge of computing it? It's not a cheap thing to recompute retroactively,
especially for an imported type, and it seems like it's easily derived from the
things that go into DerivedData.
v.g.vassilev created this revision.
Patch by Bernd Schmidt!
Fixes PR19668 and PR23034.
Repository:
rL LLVM
https://reviews.llvm.org/D35056
Files:
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
test/CodeGenCXX/uncopyable-args.cpp
Index: test/CodeGenCXX/uncopyable-args.cpp
===
28 matches
Mail list logo
