rnk created this revision.
rnk added a reviewer: hans.
I don't like this change, because I had to copy a fair amount of logic
from RAV into the dllimport visitor. However, the build time and size
results were still impressive, so I wanted to upload this and get a
second opinion. Here's the data:
| metric | before | after | diff |
| time (s) | 38.7 | 25.3 | -13.4 |
| obj (kb) | 12012 | 9428 | -2584 |
| exe (kb) | 86000 | 85924 | -76 |
|
Do you think I should land this as is? I'm considering factoring some of
this stuff up into a "RecursiveStmtVisitor" that tries to visit "code
that executes in a particular function scope". It could be used for
availability diagnostics that search for use of particular local
variables, etc. That seems to be the main use case we have for RAVs:
given a predicate, find all referenced decls, and check some predicate.
https://reviews.llvm.org/D57268
Files:
clang/lib/CodeGen/CodeGenModule.cpp
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -32,7 +32,6 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Mangle.h"
#include "clang/AST/RecordLayout.h"
-#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/CharInfo.h"
@@ -2317,64 +2316,95 @@
// Make sure we're not referencing non-imported vars or functions.
struct DLLImportFunctionVisitor
- : public RecursiveASTVisitor<DLLImportFunctionVisitor> {
- bool SafeToInline = true;
-
- bool shouldVisitImplicitCode() const { return true; }
-
- bool VisitVarDecl(VarDecl *VD) {
- if (VD->getTLSKind()) {
- // A thread-local variable cannot be imported.
- SafeToInline = false;
- return SafeToInline;
+ : public ConstStmtVisitor<DLLImportFunctionVisitor, bool> {
+ bool VisitDeclStmt(const DeclStmt *E) {
+ for (const Decl *D : E->decls()) {
+ if (const auto *VD = dyn_cast<VarDecl>(D)) {
+ if (!VisitVarDecl(VD))
+ return false;
+ // Traverse variable initializers, which are common sources of
+ // function references.
+ if (!VisitStmt(VD->getInit()))
+ return false;
+ }
}
+ return true;
+ }
+
+ bool VisitVarDecl(const VarDecl *VD) {
+ // A thread-local variable cannot be imported.
+ if (VD->getTLSKind())
+ return false;
// A variable definition might imply a destructor call.
if (VD->isThisDeclarationADefinition())
- SafeToInline = !HasNonDllImportDtor(VD->getType());
+ if (HasNonDllImportDtor(VD->getType()))
+ return false;
- return SafeToInline;
+ return true;
}
- bool VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) {
+ bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E) {
+ bool SafeToInline = true;
if (const auto *D = E->getTemporary()->getDestructor())
SafeToInline = D->hasAttr<DLLImportAttr>();
return SafeToInline;
}
- bool VisitDeclRefExpr(DeclRefExpr *E) {
- ValueDecl *VD = E->getDecl();
+ bool VisitDeclRefExpr(const DeclRefExpr *E) {
+ const ValueDecl *VD = E->getDecl();
+ bool SafeToInline = true;
if (isa<FunctionDecl>(VD))
SafeToInline = VD->hasAttr<DLLImportAttr>();
- else if (VarDecl *V = dyn_cast<VarDecl>(VD))
+ else if (const VarDecl *V = dyn_cast<VarDecl>(VD))
SafeToInline = !V->hasGlobalStorage() || V->hasAttr<DLLImportAttr>();
return SafeToInline;
}
- bool VisitCXXConstructExpr(CXXConstructExpr *E) {
- SafeToInline = E->getConstructor()->hasAttr<DLLImportAttr>();
- return SafeToInline;
+ bool VisitCXXConstructExpr(const CXXConstructExpr *E) {
+ return E->getConstructor()->hasAttr<DLLImportAttr>();
}
- bool VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
- CXXMethodDecl *M = E->getMethodDecl();
- if (!M) {
- // Call through a pointer to member function. This is safe to inline.
- SafeToInline = true;
- } else {
+ bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *E) {
+ // If M is null, this is a call through a pointer to member function. This
+ // is safe to inline.
+ const CXXMethodDecl *M = E->getMethodDecl();
+ bool SafeToInline = true;
+ if (M)
SafeToInline = M->hasAttr<DLLImportAttr>();
- }
return SafeToInline;
}
- bool VisitCXXDeleteExpr(CXXDeleteExpr *E) {
- SafeToInline = E->getOperatorDelete()->hasAttr<DLLImportAttr>();
- return SafeToInline;
+ bool VisitCXXDeleteExpr(const CXXDeleteExpr *E) {
+ return E->getOperatorDelete()->hasAttr<DLLImportAttr>();
}
- bool VisitCXXNewExpr(CXXNewExpr *E) {
- SafeToInline = E->getOperatorNew()->hasAttr<DLLImportAttr>();
- return SafeToInline;
+ bool VisitCXXNewExpr(const CXXNewExpr *E) {
+ return E->getOperatorNew()->hasAttr<DLLImportAttr>();
+ }
+
+ bool VisitStmt(const Stmt *S) {
+ if (!S)
+ return true;
+ for (const Stmt *Child : S->children())
+ if (Child && !this->Visit(Child))
+ return false;
+ return true;
+ }
+
+ static bool isSafeToInline(const FunctionDecl *FD) {
+ DLLImportFunctionVisitor Visitor;
+ if (!Visitor.Visit(FD->getBody()))
+ return false;
+
+ // We also need to check constructor initializers.
+ if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(FD)) {
+ for (auto *I : Ctor->inits()) {
+ if (!Visitor.Visit(I->getInit()))
+ return false;
+ }
+ }
+ return true;
}
};
}
@@ -2409,9 +2439,7 @@
if (F->hasAttr<DLLImportAttr>()) {
// Check whether it would be safe to inline this dllimport function.
- DLLImportFunctionVisitor Visitor;
- Visitor.TraverseFunctionDecl(const_cast<FunctionDecl*>(F));
- if (!Visitor.SafeToInline)
+ if (!DLLImportFunctionVisitor::isSafeToInline(F))
return false;
if (const CXXDestructorDecl *Dtor = dyn_cast<CXXDestructorDecl>(F)) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
