This revision was automatically updated to reflect the committed changes.
Closed by commit rL327939: [CodeGen] Ignore OpaqueValueExprs that are unique
references to their (authored by ahatanak, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llv
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Alright, LGTM.
https://reviews.llvm.org/D39562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
ahatanak added a comment.
I also add a call to setIsUnique(false) in OpaqueValueExpr's constructor, which
fixed the failing tests I was seeing.
https://reviews.llvm.org/D39562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
ahatanak updated this revision to Diff 137418.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Don't set the IsUnique bit of an OpaqueValueExpr that is used as the result
expression.
https://reviews.llvm.org/D39562
Files:
include/clang/AST/Expr.h
include/clang/AST/Stmt
ahatanak added inline comments.
Comment at: lib/CodeGen/CGExpr.cpp:4815
+}
+ }
+
rjmccall wrote:
> Oh! So it's an interesting point that the RHS might be used as the result
> expression, which means its use might not really be unique anymore. It
> happen
ahatanak added a comment.
Sorry, this patch is not ready yet. There are several regression tests that are
failing because of the assert in setIsUnique.
https://reviews.llvm.org/D39562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://l
rjmccall added inline comments.
Comment at: include/clang/AST/Expr.h:875
+ /// is set to true.
+ bool IsUnique = false;
+
rjmccall wrote:
> Humor me and pack this in the bitfields in Stmt, please. :)
Thanks!
Comment at: lib/CodeGen/CGExpr.cpp
ahatanak added inline comments.
Comment at: lib/Sema/SemaPseudoObject.cpp:432
+ if (capturedRHS->getType()->getAsCXXRecordDecl() && capturedRHS->isRValue())
+capturedRHS->setIsUnique();
+
rjmccall wrote:
> I think you can unconditionally set this here, actua
ahatanak updated this revision to Diff 137250.
ahatanak marked 3 inline comments as done.
ahatanak added a comment.
Address review comments.
https://reviews.llvm.org/D39562
Files:
include/clang/AST/Expr.h
include/clang/AST/Stmt.h
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprAgg.cpp
lib/Co
rjmccall added a comment.
Oh, and you need to serialize this bit.
https://reviews.llvm.org/D39562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rjmccall added inline comments.
Comment at: include/clang/AST/Expr.h:875
+ /// is set to true.
+ bool IsUnique = false;
+
Humor me and pack this in the bitfields in Stmt, please. :)
Comment at: include/clang/AST/Expr.h:932
+ void setIsUniq
ahatanak updated this revision to Diff 136895.
ahatanak added a comment.
Add a flag to OpaqeuValueExpr that indicates whether it is a unique reference
to its subexpression. If the flag is set, IRGen will avoid binding the OVE and
copying the result to the destination and instead just visit the O
ahatanak added a comment.
I'm planning to work on a new patch this week.
https://reviews.llvm.org/D39562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
smeenai added a comment.
Any updates here? We were hitting a similar error internally, which this patch
fixed.
https://reviews.llvm.org/D39562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
rjmccall added a comment.
Hmm. The OVE-ing of the RHS of a property assignment is just there to make the
original source structure clearer. Maybe the right solution here is to set a
flag in the OVE that says that it's a unique semantic reference to its source
expression, and then change IRGen
ahatanak added a comment.
This is tracked by rdar://problem/34363596.
https://reviews.llvm.org/D39562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ahatanak created this revision.
Herald added subscribers: kristof.beyls, aemerson.
The assertion failure occurs when a setter is called using the dot syntax to
set a property of a class type that doesn't have trivial copy/move constructors
or assignment operators. It happens only when clang is c
17 matches
Mail list logo