Skip to content

Commit ec11e63

Browse files
PetroZarytskyivgvassilev
authored andcommitted
Switch from Visit to Traverse functions in TBR.
Currently, `TBRAnalyzer` is implemented as a `RecursiveASTVisitor`, meaning it automatically visits all sub-stmts recursively. However, in TBR, we often need to visit sub-stmts in a specific way, e.g. When analyzing ``` x = y; ``` we should visit ``x`` with settings according to the fact that ``x`` is being overwritten, and then reset the settings. The same goes for ``y``. Therefore, we cannot perform the analysis in the order, in which `RecursiveASTVisitor` visits this stmt: `x = y`, then `x`, then `y`. Instead, we call ``TraverseStmt`` manually inside ``VisitBinaryOperator``. Because of this we traverse stmts more times than we need to: ``` 1) x = y; -> also x and y manually 2) x -> automatically, unnecessary 3) y -> automatically, unnecessary ``` We can show that for nested stmts, this will bump complexity from linear to quadratic. The same problem also occurs for other stmts like ``CallExpr``, where we visit the ``DeclRefExpr`` of the function. This can be solved by switching from ``Visit`` functions to ``Traverse`` and returning ``false`` to prevent the visitor from entering the child nodes (returning ``false`` in ``Visit`` functions stops the whole visitation). The only downside is that ``Traverse`` only works with exact node type matches, e.g. ``TraverseCallExpr`` is not called for a ``CXXMemberCallExpr`` even though it's a subclass of ``CallExpr``. We can solve this by introducing dummy functions like ``` TBRAnalyzer::TraverseCXXMemberCallExpr(const CXXMemberCallExpr* MCE) { TBRAnalyzer::TraverseCallExpr(MCE); return false; } ```
1 parent 4a00a64 commit ec11e63

2 files changed

Lines changed: 51 additions & 35 deletions

File tree

lib/Differentiator/TBRAnalyzer.cpp

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -264,11 +264,6 @@ void TBRAnalyzer::markLocation(const clang::Expr* E) {
264264
void TBRAnalyzer::setIsRequired(const clang::Expr* E, bool isReq) {
265265
// FIXME: generalize to other exprs
266266
if (const auto* DRE = dyn_cast<DeclRefExpr>(E)) {
267-
// Since TBRAnalyzer is a RecursiveASTVisitor,
268-
// it automatically visits all sub-stmts including
269-
// decl refs of functions.
270-
if (!isa<VarDecl>(DRE->getDecl()))
271-
return;
272267
const auto* VD = cast<VarDecl>(DRE->getDecl());
273268
auto& curBranch = getCurBlockVarsData();
274269
if (curBranch.find(VD) == curBranch.end()) {
@@ -566,12 +561,12 @@ void TBRAnalyzer::merge(VarsData* targetData, VarsData* mergeData) {
566561
}
567562
}
568563

569-
bool TBRAnalyzer::VisitDeclRefExpr(DeclRefExpr* DRE) {
564+
bool TBRAnalyzer::TraverseDeclRefExpr(DeclRefExpr* DRE) {
570565
setIsRequired(DRE);
571-
return true;
566+
return false;
572567
}
573568

574-
bool TBRAnalyzer::VisitDeclStmt(DeclStmt* DS) {
569+
bool TBRAnalyzer::TraverseDeclStmt(DeclStmt* DS) {
575570
for (auto* D : DS->decls()) {
576571
if (auto* VD = dyn_cast<VarDecl>(D)) {
577572
addVar(VD);
@@ -590,10 +585,10 @@ bool TBRAnalyzer::VisitDeclStmt(DeclStmt* DS) {
590585
}
591586
}
592587
}
593-
return true;
588+
return false;
594589
}
595590

596-
bool TBRAnalyzer::VisitConditionalOperator(clang::ConditionalOperator* CO) {
591+
bool TBRAnalyzer::TraverseConditionalOperator(clang::ConditionalOperator* CO) {
597592
setMode(0);
598593
TraverseStmt(CO->getCond());
599594
resetMode();
@@ -609,10 +604,10 @@ bool TBRAnalyzer::VisitConditionalOperator(clang::ConditionalOperator* CO) {
609604
TraverseStmt(CO->getTrueExpr());
610605

611606
merge(m_BlockData[m_CurBlockID].get(), thenBranch.get());
612-
return true;
607+
return false;
613608
}
614609

615-
bool TBRAnalyzer::VisitBinaryOperator(BinaryOperator* BinOp) {
610+
bool TBRAnalyzer::TraverseBinaryOperator(BinaryOperator* BinOp) {
616611
const auto opCode = BinOp->getOpcode();
617612
Expr* L = BinOp->getLHS();
618613
Expr* R = BinOp->getRHS();
@@ -705,10 +700,16 @@ bool TBRAnalyzer::VisitBinaryOperator(BinaryOperator* BinOp) {
705700
// else {
706701
// FIXME: add logic/bitwise/comparison operators
707702
// }
708-
return true;
703+
return false;
709704
}
710705

711-
bool TBRAnalyzer::VisitUnaryOperator(clang::UnaryOperator* UnOp) {
706+
bool TBRAnalyzer::TraverseCompoundAssignOperator(
707+
clang::CompoundAssignOperator* BinOp) {
708+
TBRAnalyzer::TraverseBinaryOperator(BinOp);
709+
return false;
710+
}
711+
712+
bool TBRAnalyzer::TraverseUnaryOperator(clang::UnaryOperator* UnOp) {
712713
const auto opCode = UnOp->getOpcode();
713714
Expr* E = UnOp->getSubExpr();
714715
TraverseStmt(E);
@@ -733,10 +734,10 @@ bool TBRAnalyzer::VisitUnaryOperator(clang::UnaryOperator* UnOp) {
733734
// expressions. However, it is not clear where the FieldDecls of real and
734735
// imaginary parts should be deduced from (their names might be
735736
// compiler-specific). So for now we visit the whole subexpression.
736-
return true;
737+
return false;
737738
}
738739

739-
bool TBRAnalyzer::VisitCallExpr(clang::CallExpr* CE) {
740+
bool TBRAnalyzer::TraverseCallExpr(clang::CallExpr* CE) {
740741
// FIXME: Currently TBR analysis just stops here and assumes that all the
741742
// variables passed by value/reference are used/used and changed. Analysis
742743
// could proceed to the function to analyse data flow inside it.
@@ -769,10 +770,20 @@ bool TBRAnalyzer::VisitCallExpr(clang::CallExpr* CE) {
769770
}
770771
}
771772
resetMode();
772-
return true;
773+
return false;
773774
}
774775

775-
bool TBRAnalyzer::VisitCXXConstructExpr(clang::CXXConstructExpr* CE) {
776+
bool TBRAnalyzer::TraverseCXXMemberCallExpr(clang::CXXMemberCallExpr* CE) {
777+
TBRAnalyzer::TraverseCallExpr(CE);
778+
return false;
779+
}
780+
781+
bool TBRAnalyzer::TraverseCXXOperatorCallExpr(clang::CXXOperatorCallExpr* CE) {
782+
TBRAnalyzer::TraverseCallExpr(CE);
783+
return false;
784+
}
785+
786+
bool TBRAnalyzer::TraverseCXXConstructExpr(clang::CXXConstructExpr* CE) {
776787
// FIXME: Currently TBR analysis just stops here and assumes that all the
777788
// variables passed by value/reference are used/used and changed. Analysis
778789
// could proceed to the constructor to analyse data flow inside it.
@@ -796,31 +807,31 @@ bool TBRAnalyzer::VisitCXXConstructExpr(clang::CXXConstructExpr* CE) {
796807
}
797808
}
798809
resetMode();
799-
return true;
810+
return false;
800811
}
801812

802-
bool TBRAnalyzer::VisitMemberExpr(clang::MemberExpr* ME) {
813+
bool TBRAnalyzer::TraverseMemberExpr(clang::MemberExpr* ME) {
803814
setIsRequired(ME);
804-
return true;
815+
return false;
805816
}
806817

807-
bool TBRAnalyzer::VisitArraySubscriptExpr(clang::ArraySubscriptExpr* ASE) {
818+
bool TBRAnalyzer::TraverseArraySubscriptExpr(clang::ArraySubscriptExpr* ASE) {
808819
setMode(0);
809820
TraverseStmt(ASE->getBase());
810821
resetMode();
811822
setIsRequired(ASE);
812823
setMode(Mode::kMarkingMode | Mode::kNonLinearMode);
813824
TraverseStmt(ASE->getIdx());
814825
resetMode();
815-
return true;
826+
return false;
816827
}
817828

818-
bool TBRAnalyzer::VisitInitListExpr(clang::InitListExpr* ILE) {
829+
bool TBRAnalyzer::TraverseInitListExpr(clang::InitListExpr* ILE) {
819830
setMode(Mode::kMarkingMode);
820831
for (auto* init : ILE->inits())
821832
TraverseStmt(init);
822833
resetMode();
823-
return true;
834+
return false;
824835
}
825836

826837
} // end namespace clad

lib/Differentiator/TBRAnalyzer.h

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
#ifndef CLAD_DIFFERENTIATOR_TBRANALYZER_H
22
#define CLAD_DIFFERENTIATOR_TBRANALYZER_H
33

4+
#include "clang/AST/ExprCXX.h"
45
#include "clang/AST/RecursiveASTVisitor.h"
6+
#include "clang/AST/Stmt.h"
57
#include "clang/Analysis/CFG.h"
68

79
#include "clad/Differentiator/CladUtils.h"
@@ -299,16 +301,19 @@ class TBRAnalyzer : public clang::RecursiveASTVisitor<TBRAnalyzer> {
299301

300302
void VisitCFGBlock(const clang::CFGBlock& block);
301303

302-
bool VisitArraySubscriptExpr(clang::ArraySubscriptExpr* ASE);
303-
bool VisitBinaryOperator(clang::BinaryOperator* BinOp);
304-
bool VisitCallExpr(clang::CallExpr* CE);
305-
bool VisitConditionalOperator(clang::ConditionalOperator* CO);
306-
bool VisitCXXConstructExpr(clang::CXXConstructExpr* CE);
307-
bool VisitDeclRefExpr(clang::DeclRefExpr* DRE);
308-
bool VisitDeclStmt(clang::DeclStmt* DS);
309-
bool VisitInitListExpr(clang::InitListExpr* ILE);
310-
bool VisitMemberExpr(clang::MemberExpr* ME);
311-
bool VisitUnaryOperator(clang::UnaryOperator* UnOp);
304+
bool TraverseArraySubscriptExpr(clang::ArraySubscriptExpr* ASE);
305+
bool TraverseBinaryOperator(clang::BinaryOperator* BinOp);
306+
bool TraverseCallExpr(clang::CallExpr* CE);
307+
bool TraverseConditionalOperator(clang::ConditionalOperator* CO);
308+
bool TraverseCompoundAssignOperator(clang::CompoundAssignOperator* BinOp);
309+
bool TraverseCXXConstructExpr(clang::CXXConstructExpr* CE);
310+
bool TraverseCXXMemberCallExpr(clang::CXXMemberCallExpr* CE);
311+
bool TraverseCXXOperatorCallExpr(clang::CXXOperatorCallExpr* CE);
312+
bool TraverseDeclRefExpr(clang::DeclRefExpr* DRE);
313+
bool TraverseDeclStmt(clang::DeclStmt* DS);
314+
bool TraverseInitListExpr(clang::InitListExpr* ILE);
315+
bool TraverseMemberExpr(clang::MemberExpr* ME);
316+
bool TraverseUnaryOperator(clang::UnaryOperator* UnOp);
312317
};
313318

314319
} // end namespace clad

0 commit comments

Comments
 (0)