Make built-in type variable declarations global #738
Conversation
|
clang-tidy review says "All clean, LGTM! 👍" |
PetroZarytskyi
force-pushed
the
global-vars
branch
from
6da2b92 to
f4f8b5d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #738 +/- ##
==========================================
+ Coverage 94.60% 94.63% +0.03%
==========================================
Files 49 49
Lines 7190 7201 +11
==========================================
+ Hits 6802 6815 +13
+ Misses 388 386 -2
|
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Why do we need both this pull request and #688? |
PetroZarytskyi
force-pushed
the
global-vars
branch
from
f4f8b5d to
b1b24a2
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
PetroZarytskyi
force-pushed
the
global-vars
branch
from
b1b24a2 to
7ebcff1
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
PetroZarytskyi
force-pushed
the
global-vars
branch
from
7ebcff1 to
2776e59
Compare
vgvassilev
force-pushed
the
global-vars
branch
from
03efa6b to
9fa1bf8
Compare
PetroZarytskyi
force-pushed
the
global-vars
branch
from
9642d20 to
ce70387
Compare
| @@ -2530,19 +2516,26 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, | |||
| VarDeclDiff ReverseModeVisitor::DifferentiateVarDecl(const VarDecl* VD) { | |||
| StmtDiff initDiff; | |||
| Expr* VDDerivedInit = nullptr; | |||
| // We take the parent of the current scope because the main compound | |||
| // statement of the function has its own scope as well. | |||
| bool isInFunctionGlobalScope = getCurrentScope()->getParent()==m_DerivativeFnScope; | |||
There was a problem hiding this comment.
| bool isInFunctionGlobalScope = getCurrentScope()->getParent()==m_DerivativeFnScope; | |
| bool isFunctionScope = getCurrentScope()->isFunctionScope(); |
Any clue why that does not work?
There was a problem hiding this comment.
Because the outermost compound statement is not considered function scope.
There was a problem hiding this comment.
I made the outermost compound statement FnScope so now this option works. I did this in my last commit.
| @@ -2530,19 +2516,26 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, | |||
| VarDeclDiff ReverseModeVisitor::DifferentiateVarDecl(const VarDecl* VD) { | |||
| StmtDiff initDiff; | |||
| Expr* VDDerivedInit = nullptr; | |||
| // We take the parent of the current scope because the main compound | |||
There was a problem hiding this comment.
If getCurrentScope()->isFunctionScope() is false for int f() { this probably means that when we create the scope for the first compound statement of the function we should also add the Scope::FnScope flag to the current scope.
There was a problem hiding this comment.
This is a good idea. I did this in my last commit. Thank you for pointing this out.
| @@ -2752,6 +2725,9 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, | |||
| llvm::SmallVector<Decl*, 4> declsDiff; | |||
| // Need to put array decls inlined. | |||
| llvm::SmallVector<Decl*, 4> localDeclsDiff; | |||
| // We take the parent of the current scope because the main compound | |||
| // statement of the function has its own scope as well. | |||
| bool isInFunctionGlobalScope = getCurrentScope()->getParent()==m_DerivativeFnScope; | |||
|
clang-tidy review says "All clean, LGTM! 👍" |
3 similar comments
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
When we produce a gradient function we generally have a forward and reverse sweep. In the forward sweep we accumulate the state and in the reverse sweep we use that state to invert the program execution. The forward sweep generally follows the sematics of the primal function and when neccessary stores the state which would be needed but lost for the reverse sweep. However, to minimize the stores onto the tape we need to reuse some of the variables between the forward and the reverse sweeps which requires some variables to be promoted to the enclosing lexical scope of both sweeps. Fixes vgvassilev#659, fixes vgvassilev#681.
vgvassilev
force-pushed
the
global-vars
branch
from
e46cbef to
d225ab6
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
When we produce a gradient function we generally have a forward and reverse
sweep. In the forward sweep we accumulate the state and in the reverse sweep
we use that state to invert the program execution. The forward sweep generally
follows the sematics of the primal function and when neccessary stores the state
which would be needed but lost for the reverse sweep.
However, to minimize the stores onto the tape we need to reuse some of the
variables between the forward and the reverse sweeps which requires some
variables to be promoted to the enclosing lexical scope of both sweeps.
Fixes #659, fixes #681.