Add TBR analysis in the reverse mode of Clad #616
Conversation
This patch defers the decisions to push and pop values to the upper visitor operations which have more context and can decide if a value is worth storing or not.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // ConstExprUsage Usage, ASTContext &) | ||
| // => bool Expr::EvaluateAsConstantExpr(EvalResult &Result, ASTContext &) | ||
|
|
||
| static inline bool Expr_EvaluateAsConstantExpr(const Expr* E, |
There was a problem hiding this comment.
warning: unused function 'Expr_EvaluateAsConstantExpr' [clang-diagnostic-unused-function]
static inline bool Expr_EvaluateAsConstantExpr(const Expr* E,
^| else if (d == direction::reverse) | ||
| return m_Reverse.back(); | ||
| else | ||
| return m_EssentialReverse.back(); |
There was a problem hiding this comment.
warning: do not use 'else' after 'return' [llvm-else-after-return]
| else if (d == direction::reverse) | |
| return m_Reverse.back(); | |
| else | |
| return m_EssentialReverse.back(); | |
| if (d == direction::reverse) | |
| return m_Reverse.back(); | |
| else | |
| return m_EssentialReverse.back(); |
| m_Reverse.push_back({}); | ||
| else | ||
| m_EssentialReverse.push_back({}); |
There was a problem hiding this comment.
warning: use emplace_back instead of push_back [modernize-use-emplace]
| m_EssentialReverse.push_back({}); | |
| m_EssentialReverse.emplace_back(); |
| } else if (d == direction::reverse) { | ||
| auto CS = MakeCompoundStmt(getCurrentBlock(direction::reverse)); | ||
| std::reverse(CS->body_begin(), CS->body_end()); | ||
| m_Reverse.pop_back(); | ||
| return CS; | ||
| } else { | ||
| auto CS = MakeCompoundStmt(getCurrentBlock(d)); | ||
| m_EssentialReverse.pop_back(); | ||
| return CS; | ||
| } |
There was a problem hiding this comment.
warning: do not use 'else' after 'return' [llvm-else-after-return]
| } else if (d == direction::reverse) { | |
| auto CS = MakeCompoundStmt(getCurrentBlock(direction::reverse)); | |
| std::reverse(CS->body_begin(), CS->body_end()); | |
| m_Reverse.pop_back(); | |
| return CS; | |
| } else { | |
| auto CS = MakeCompoundStmt(getCurrentBlock(d)); | |
| m_EssentialReverse.pop_back(); | |
| return CS; | |
| } | |
| } if (d == direction::reverse) { | |
| auto CS = MakeCompoundStmt(getCurrentBlock(direction::reverse)); | |
| std::reverse(CS->body_begin(), CS->body_end()); | |
| m_Reverse.pop_back(); | |
| return CS; | |
| } else { | |
| auto CS = MakeCompoundStmt(getCurrentBlock(d)); | |
| m_EssentialReverse.pop_back(); | |
| return CS; | |
| } |
| auto CS = MakeCompoundStmt(getCurrentBlock(direction::reverse)); | ||
| std::reverse(CS->body_begin(), CS->body_end()); | ||
| m_Reverse.pop_back(); | ||
| return CS; | ||
| } else { | ||
| auto CS = MakeCompoundStmt(getCurrentBlock(d)); |
There was a problem hiding this comment.
warning: 'auto CS' can be declared as 'auto *CS' [llvm-qualified-auto]
auto CS = MakeCompoundStmt(getCurrentBlock(d));
^this fix will not be applied because it overlaps with another fix
| @@ -553,7 +583,9 @@ | |||
| void PopBreakContStmtHandler() { | |||
| m_BreakContStmtHandlers.pop_back(); | |||
| } | |||
|
|
|||
|
|
|||
| std::map<clang::SourceLocation, bool> m_ToBeRecorded; | |||
There was a problem hiding this comment.
warning: member variable 'm_ToBeRecorded' has public visibility [cppcoreguidelines-non-private-member-variables-in-classes]
std::map<clang::SourceLocation, bool> m_ToBeRecorded;
^| #ifndef CLAD_TBR_ANALYZER_H | ||
| #define CLAD_TBR_ANALYZER_H |
There was a problem hiding this comment.
warning: header guard does not follow preferred style [llvm-header-guard]
| #ifndef CLAD_TBR_ANALYZER_H | |
| #define CLAD_TBR_ANALYZER_H | |
| #ifndef CLAD_DIFFERENTIATOR_TBRANALYZER_H | |
| #define CLAD_DIFFERENTIATOR_TBRANALYZER_H |
include/clad/Differentiator/TBRAnalyzer.h:303:
- #endif // CLAD_TBR_ANALYZER_H
+ #endif // CLAD_DIFFERENTIATOR_TBRANALYZER_H|
|
||
| namespace clad { | ||
|
|
||
| class TBRAnalyzer : public clang::ConstStmtVisitor<TBRAnalyzer> { |
There was a problem hiding this comment.
warning: class 'TBRAnalyzer' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class TBRAnalyzer : public clang::ConstStmtVisitor<TBRAnalyzer> {
^| /// type keys. | ||
| struct APIntHash { | ||
| size_t operator()(const llvm::APInt& apint) const { | ||
| return std::hash<std::string>{}(apint.toString(10, true)); |
There was a problem hiding this comment.
warning: 10 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]
return std::hash<std::string>{}(apint.toString(10, true));
^| /// Just a helper struct serving as a wrapper for IdxOrMemberValue union. | ||
| /// Used to unwrap expressions like a[6].x.t[3]. Only used in | ||
| /// TBRAnalyzer::overlay(). | ||
| struct IdxOrMember { |
There was a problem hiding this comment.
warning: class 'IdxOrMember' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
struct IdxOrMember {
^
This pull request adds TBR (To-BeRecorded) analysis in the reverse mode of Clad. There are still that issues must be addressed before merging this to the master branch.