Re: [PATCH] D12366: Avoid unnecessarily storing vtable pointers in more destructor cases

2015-08-27 Thread scott douglass via cfe-commits
scott-0 added a comment.

Thanks for the review and advice; I'll give `undef` a try.  It's a much simpler 
approach.


http://reviews.llvm.org/D12366



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D12366: Avoid unnecessarily storing vtable pointers in more destructor cases

2015-08-26 Thread Richard Smith via cfe-commits
rsmith added a subscriber: rsmith.


Comment at: lib/CodeGen/CGClass.cpp:1369-1382
@@ +1368,16 @@
+
+void VisitCallExpr(const Expr *E) {
+  Sensitive = true;
+}
+void VisitCXXTypeidExpr(const Expr *E) {
+  Sensitive = true;
+}
+void VisitExpr(const Expr *E) {
+  if (E-getStmtClass() == Stmt::CXXTypeidExprClass ||
+  E-getStmtClass() == Stmt::CXXDynamicCastExprClass) {
+Sensitive = true;
+  }
+  if (!Sensitive)
+Inherited::VisitExpr(E);
+}
+  };

This is not complete; you'll also need to at least consider atomic stores (that 
might store the `this` pointer and be picked up by another thread), the 
implicit call to `operator new` in a `CXXNewExpr` (which is not modeled as a 
`CallExpr`), implicit destructor calls (which aren't modeled in the AST at 
all), and probably a lot of other cases.

Maybe it would be simpler to store `undef` to the vptr immediately before 
calling the base class destructor, and let the LLVM optimization passes remove 
this vptr store as dead if it can prove the vptr is unused?


http://reviews.llvm.org/D12366



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits