From 4ecf347301e0d85d5ba59fa39db54603c8150064 Mon Sep 17 00:00:00 2001 From: Vaibhav Thakkar Date: Sun, 18 Feb 2024 20:13:26 +0100 Subject: [PATCH 1/6] Initial support for new and delete operations in reverse mode --- .../clad/Differentiator/ReverseModeVisitor.h | 4 ++ lib/Differentiator/ReverseModeVisitor.cpp | 63 +++++++++++++++- test/Gradient/Pointers.C | 72 ++++++++++++++++++- 3 files changed, 137 insertions(+), 2 deletions(-) diff --git a/include/clad/Differentiator/ReverseModeVisitor.h b/include/clad/Differentiator/ReverseModeVisitor.h index 1cd1c0bfa..b38749bf1 100644 --- a/include/clad/Differentiator/ReverseModeVisitor.h +++ b/include/clad/Differentiator/ReverseModeVisitor.h @@ -41,6 +41,8 @@ namespace clad { /// the reverse mode we also accumulate Stmts for the reverse pass which /// will be executed on return. std::vector m_Reverse; + /// Storing expressions to delete/free memory in the reverse pass. + Stmts m_DeallocExprs; /// Stack is used to pass the arguments (dfdx) to further nodes /// in the Visit method. std::stack m_Stack; @@ -370,6 +372,8 @@ namespace clad { StmtDiff VisitContinueStmt(const clang::ContinueStmt* CS); StmtDiff VisitBreakStmt(const clang::BreakStmt* BS); StmtDiff VisitCXXThisExpr(const clang::CXXThisExpr* CTE); + StmtDiff VisitCXXNewExpr(const clang::CXXNewExpr* CNE); + StmtDiff VisitCXXDeleteExpr(const clang::CXXDeleteExpr* CDE); StmtDiff VisitCXXConstructExpr(const clang::CXXConstructExpr* CE); StmtDiff VisitMaterializeTemporaryExpr(const clang::MaterializeTemporaryExpr* MTE); diff --git a/lib/Differentiator/ReverseModeVisitor.cpp b/lib/Differentiator/ReverseModeVisitor.cpp index 6e3f1af48..a9529270c 100644 --- a/lib/Differentiator/ReverseModeVisitor.cpp +++ b/lib/Differentiator/ReverseModeVisitor.cpp @@ -610,6 +610,13 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, addToCurrentBlock(S, direction::forward); else addToCurrentBlock(Reverse, direction::forward); + // Add delete statements present in m_DeallocExprs to the current block. + for (auto* S : m_DeallocExprs) + if (auto* CS = dyn_cast(S)) + for (Stmt* S : CS->body()) + addToCurrentBlock(S, direction::forward); + else + addToCurrentBlock(S, direction::forward); if (m_ExternalSource) m_ExternalSource->ActOnEndOfDerivedFnBody(); @@ -2573,6 +2580,13 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, bool isDerivativeOfRefType = VD->getType()->isReferenceType(); VarDecl* VDDerived = nullptr; bool isPointerType = VD->getType()->isPointerType(); + bool isInitializedByNewExpr = false; + // Check if the variable is pointer type and initialized by new expression + if (isPointerType && VD->getInit()) { + if (isa(VD->getInit())) { + isInitializedByNewExpr = true; + } + } // VDDerivedInit now serves two purposes -- as the initial derivative value // or the size of the derivative array -- depending on the primal type. @@ -2663,8 +2677,12 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, // differentiated and should not be differentiated again. // If `VD` is a reference to a non-local variable then also there's no // need to call `Visit` since non-local variables are not differentiated. - if (!isDerivativeOfRefType && !isPointerType) { + if (!isDerivativeOfRefType && !(isPointerType && !isInitializedByNewExpr)) { Expr* derivedE = BuildDeclRef(VDDerived); + if (isInitializedByNewExpr) { + // derivedE should be dereferenced. + derivedE = BuildOp(UnaryOperatorKind::UO_Deref, derivedE); + } if (VD->getInit()) { if (isa(VD->getInit())) initDiff = Visit(VD->getInit()); @@ -3703,6 +3721,49 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, return {clonedCTE, m_ThisExprDerivative}; } + StmtDiff ReverseModeVisitor::VisitCXXNewExpr(const clang::CXXNewExpr* CNE) { + StmtDiff initializerDiff; + if (CNE->hasInitializer()) + initializerDiff = Visit(CNE->getInitializer(), dfdx()); + + Expr* clonedArraySizeE = nullptr; + Expr* derivedArraySizeE = nullptr; + if (CNE->getArraySize()) { + clonedArraySizeE = + Visit(clad_compat::ArraySize_GetValue(CNE->getArraySize())).getExpr(); + // Array size is a non-differentiable expression, thus the original value + // should be used in both the cloned and the derived statements. + derivedArraySizeE = Clone(clonedArraySizeE); + } + Expr* clonedNewE = utils::BuildCXXNewExpr( + m_Sema, CNE->getAllocatedType(), clonedArraySizeE, + initializerDiff.getExpr(), CNE->getAllocatedTypeSourceInfo()); + Expr* derivedNewE = utils::BuildCXXNewExpr( + m_Sema, CNE->getAllocatedType(), derivedArraySizeE, + initializerDiff.getExpr_dx(), CNE->getAllocatedTypeSourceInfo()); + return {clonedNewE, derivedNewE}; +} + +StmtDiff +ReverseModeVisitor::VisitCXXDeleteExpr(const clang::CXXDeleteExpr* CDE) { + StmtDiff argDiff = Visit(CDE->getArgument()); + Expr* clonedDeleteE = + m_Sema + .ActOnCXXDelete(noLoc, CDE->isGlobalDelete(), CDE->isArrayForm(), + argDiff.getExpr()) + .get(); + Expr* derivedDeleteE = + m_Sema + .ActOnCXXDelete(noLoc, CDE->isGlobalDelete(), CDE->isArrayForm(), + argDiff.getExpr_dx()) + .get(); + // create a compound statement containing both the cloned and the derived + // delete expressions. + CompoundStmt* CS = MakeCompoundStmt({clonedDeleteE, derivedDeleteE}); + m_DeallocExprs.push_back(CS); + return {nullptr, nullptr}; +} + // FIXME: Add support for differentiating calls to constructors. // We currently assume that constructor arguments are non-differentiable. StmtDiff diff --git a/test/Gradient/Pointers.C b/test/Gradient/Pointers.C index 107254ae0..11a66705c 100644 --- a/test/Gradient/Pointers.C +++ b/test/Gradient/Pointers.C @@ -339,6 +339,71 @@ double pointerMultipleParams(const double* a, const double* b) { // CHECK-NEXT: _d_b[2] += _d_sum; // CHECK-NEXT: } +double newAndDeletePointer(double i, double j) { + double *p = new double(i); + double *q = new double(j); + double *r = new double[2]; + r[0] = i + j; + r[1] = i*j; + double sum = *p + *q + r[0] + r[1]; + delete p; + delete q; + delete [] r; + return sum; +} + +// CHECK: void newAndDeletePointer_grad(double i, double j, clad::array_ref _d_i, clad::array_ref _d_j) { +// CHECK-NEXT: double *_d_p = 0; +// CHECK-NEXT: double *_d_q = 0; +// CHECK-NEXT: double *_d_r = 0; +// CHECK-NEXT: double _t0; +// CHECK-NEXT: double _t1; +// CHECK-NEXT: double _d_sum = 0; +// CHECK-NEXT: _d_p = new double(* _d_i); +// CHECK-NEXT: double *p = new double(i); +// CHECK-NEXT: _d_q = new double(* _d_j); +// CHECK-NEXT: double *q = new double(j); +// CHECK-NEXT: _d_r = new double [2]; +// CHECK-NEXT: double *r = new double [2]; +// CHECK-NEXT: _t0 = r[0]; +// CHECK-NEXT: r[0] = i + j; +// CHECK-NEXT: _t1 = r[1]; +// CHECK-NEXT: r[1] = i * j; +// CHECK-NEXT: double sum = *p + *q + r[0] + r[1]; +// CHECK-NEXT: goto _label0; +// CHECK-NEXT: _label0: +// CHECK-NEXT: _d_sum += 1; +// CHECK-NEXT: { +// CHECK-NEXT: *_d_p += _d_sum; +// CHECK-NEXT: *_d_q += _d_sum; +// CHECK-NEXT: _d_r[0] += _d_sum; +// CHECK-NEXT: _d_r[1] += _d_sum; +// CHECK-NEXT: } +// CHECK-NEXT: { +// CHECK-NEXT: r[1] = _t1; +// CHECK-NEXT: double _r_d1 = _d_r[1]; +// CHECK-NEXT: _d_r[1] -= _r_d1; +// CHECK-NEXT: * _d_i += _r_d1 * j; +// CHECK-NEXT: * _d_j += i * _r_d1; +// CHECK-NEXT: } +// CHECK-NEXT: { +// CHECK-NEXT: r[0] = _t0; +// CHECK-NEXT: double _r_d0 = _d_r[0]; +// CHECK-NEXT: _d_r[0] -= _r_d0; +// CHECK-NEXT: * _d_i += _r_d0; +// CHECK-NEXT: * _d_j += _r_d0; +// CHECK-NEXT: } +// CHECK-NEXT: * _d_j += *_d_q; +// CHECK-NEXT: * _d_i += *_d_p; +// CHECK-NEXT: delete [] r; +// CHECK-NEXT: delete [] _d_r; +// CHECK-NEXT: delete q; +// CHECK-NEXT: delete _d_q; +// CHECK-NEXT: delete p; +// CHECK-NEXT: delete _d_p; +// CHECK-NEXT: } + + #define NON_MEM_FN_TEST(var)\ res[0]=0;\ var.execute(5,res);\ @@ -433,4 +498,9 @@ int main() { d_pointerMultipleParams.execute(arr, b_arr, d_arr_ref, d_b_arr_ref); printf("%.2f %.2f %.2f %.2f %.2f\n", d_arr[0], d_arr[1], d_arr[2], d_arr[3], d_arr[4]); // CHECK-EXEC: 2.00 4.00 2.00 0.00 0.00 printf("%.2f %.2f %.2f %.2f %.2f\n", d_b_arr[0], d_b_arr[1], d_b_arr[2], d_b_arr[3], d_b_arr[4]); // CHECK-EXEC: 0.00 0.00 1.00 0.00 0.00 -} + + auto d_newAndDeletePointer = clad::gradient(newAndDeletePointer); + double d_i = 0, d_j = 0; + d_newAndDeletePointer.execute(5, 7, &d_i, &d_j); + printf("%.2f %.2f\n", d_i, d_j); // CHECK-EXEC: 9.00 7.00 +} \ No newline at end of file From 3f01278c21a13c736cfd76b2b6bf8590465ef0fb Mon Sep 17 00:00:00 2001 From: Vaibhav Thakkar Date: Mon, 19 Feb 2024 15:31:24 +0100 Subject: [PATCH 2/6] Fix struct init using initializer lists --- .../Differentiator/BaseForwardModeVisitor.h | 1 + .../clad/Differentiator/ReverseModeVisitor.h | 2 + lib/Differentiator/BaseForwardModeVisitor.cpp | 5 + lib/Differentiator/ReverseModeVisitor.cpp | 117 ++++++++++-------- test/ForwardMode/Pointer.C | 25 ++++ test/Gradient/Pointers.C | 40 +++++- 6 files changed, 135 insertions(+), 55 deletions(-) diff --git a/include/clad/Differentiator/BaseForwardModeVisitor.h b/include/clad/Differentiator/BaseForwardModeVisitor.h index 375ae88d5..4ca1fcae3 100644 --- a/include/clad/Differentiator/BaseForwardModeVisitor.h +++ b/include/clad/Differentiator/BaseForwardModeVisitor.h @@ -106,6 +106,7 @@ class BaseForwardModeVisitor StmtDiff VisitPseudoObjectExpr(const clang::PseudoObjectExpr* POE); StmtDiff VisitSubstNonTypeTemplateParmExpr( const clang::SubstNonTypeTemplateParmExpr* NTTP); + StmtDiff VisitImplicitValueInitExpr(const clang::ImplicitValueInitExpr* IVIE); virtual clang::QualType GetPushForwardDerivativeType(clang::QualType ParamType); diff --git a/include/clad/Differentiator/ReverseModeVisitor.h b/include/clad/Differentiator/ReverseModeVisitor.h index b38749bf1..dff250161 100644 --- a/include/clad/Differentiator/ReverseModeVisitor.h +++ b/include/clad/Differentiator/ReverseModeVisitor.h @@ -358,6 +358,8 @@ namespace clad { StmtDiff VisitForStmt(const clang::ForStmt* FS); StmtDiff VisitIfStmt(const clang::IfStmt* If); StmtDiff VisitImplicitCastExpr(const clang::ImplicitCastExpr* ICE); + StmtDiff + VisitImplicitValueInitExpr(const clang::ImplicitValueInitExpr* IVIE); StmtDiff VisitInitListExpr(const clang::InitListExpr* ILE); StmtDiff VisitIntegerLiteral(const clang::IntegerLiteral* IL); StmtDiff VisitMemberExpr(const clang::MemberExpr* ME); diff --git a/lib/Differentiator/BaseForwardModeVisitor.cpp b/lib/Differentiator/BaseForwardModeVisitor.cpp index 37f32bf81..68979bcb3 100644 --- a/lib/Differentiator/BaseForwardModeVisitor.cpp +++ b/lib/Differentiator/BaseForwardModeVisitor.cpp @@ -1445,6 +1445,11 @@ BaseForwardModeVisitor::VisitImplicitCastExpr(const ImplicitCastExpr* ICE) { return StmtDiff(subExprDiff.getExpr(), subExprDiff.getExpr_dx()); } +StmtDiff BaseForwardModeVisitor::VisitImplicitValueInitExpr( + const ImplicitValueInitExpr* E) { + return StmtDiff(Clone(E), Clone(E)); +} + StmtDiff BaseForwardModeVisitor::VisitCXXDefaultArgExpr(const CXXDefaultArgExpr* DE) { // FIXME: Shouldn't we simply clone the CXXDefaultArgExpr? diff --git a/lib/Differentiator/ReverseModeVisitor.cpp b/lib/Differentiator/ReverseModeVisitor.cpp index a9529270c..8a010d283 100644 --- a/lib/Differentiator/ReverseModeVisitor.cpp +++ b/lib/Differentiator/ReverseModeVisitor.cpp @@ -1247,6 +1247,21 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, Expr* clonedILE = m_Sema.ActOnInitList(noLoc, clonedExprs, noLoc).get(); return StmtDiff(clonedILE); } + // Check if type is a CXXRecordDecl and a struct. + if (!isCladValueAndPushforwardType(ILEType) && ILEType->isRecordType() && + ILEType->getAsCXXRecordDecl()->isStruct()) { + for (unsigned i = 0, e = ILE->getNumInits(); i < e; i++) { + // fetch ith field of the struct. + auto field_iterator = ILEType->getAsCXXRecordDecl()->field_begin(); + std::advance(field_iterator, i); + Expr* member_acess = utils::BuildMemberExpr( + m_Sema, getCurrentScope(), dfdx(), (*field_iterator)->getName()); + clonedExprs[i] = Visit(ILE->getInit(i), member_acess).getExpr(); + } + Expr* clonedILE = m_Sema.ActOnInitList(noLoc, clonedExprs, noLoc).get(); + return StmtDiff(clonedILE); + } + // FIXME: This is a makeshift arrangement to differentiate an InitListExpr // that represents a ValueAndPushforward type. Ideally this must be // differentiated at VisitCXXConstructExpr @@ -2582,11 +2597,9 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, bool isPointerType = VD->getType()->isPointerType(); bool isInitializedByNewExpr = false; // Check if the variable is pointer type and initialized by new expression - if (isPointerType && VD->getInit()) { - if (isa(VD->getInit())) { - isInitializedByNewExpr = true; - } - } + if (isPointerType && (VD->getInit() != nullptr) && + isa(VD->getInit())) + isInitializedByNewExpr = true; // VDDerivedInit now serves two purposes -- as the initial derivative value // or the size of the derivative array -- depending on the primal type. @@ -2655,7 +2668,6 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, // if VD is a pointer type, then the initial value is set to the derived // expression of the corresponding pointer type. else if (isPointerType && VD->getInit()) { - initDiff = Visit(VD->getInit()); VDDerivedType = getNonConstType(VDDerivedType, m_Context, m_Sema); // If it's a pointer to a constant type, then remove the constness. if (VD->getType()->getPointeeType().isConstQualified()) { @@ -2677,12 +2689,10 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, // differentiated and should not be differentiated again. // If `VD` is a reference to a non-local variable then also there's no // need to call `Visit` since non-local variables are not differentiated. - if (!isDerivativeOfRefType && !(isPointerType && !isInitializedByNewExpr)) { + if (!isDerivativeOfRefType && (!isPointerType || isInitializedByNewExpr)) { Expr* derivedE = BuildDeclRef(VDDerived); - if (isInitializedByNewExpr) { - // derivedE should be dereferenced. + if (isInitializedByNewExpr) derivedE = BuildOp(UnaryOperatorKind::UO_Deref, derivedE); - } if (VD->getInit()) { if (isa(VD->getInit())) initDiff = Visit(VD->getInit()); @@ -2709,6 +2719,8 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, getZeroInit(VDDerivedType)); addToCurrentBlock(assignToZero, direction::reverse); } + } else if (isPointerType && VD->getInit()) { + initDiff = Visit(VD->getInit()); } VarDecl* VDClone = nullptr; Expr* derivedVDE = BuildDeclRef(VDDerived); @@ -2926,6 +2938,11 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, return Visit(ICE->getSubExpr(), dfdx()); } + StmtDiff ReverseModeVisitor::VisitImplicitValueInitExpr( + const ImplicitValueInitExpr* IVIE) { + return {Clone(IVIE), Clone(IVIE)}; + } + StmtDiff ReverseModeVisitor::VisitMemberExpr(const MemberExpr* ME) { auto baseDiff = VisitWithExplicitNoDfDx(ME->getBase()); auto* field = ME->getMemberDecl(); @@ -3722,47 +3739,47 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, } StmtDiff ReverseModeVisitor::VisitCXXNewExpr(const clang::CXXNewExpr* CNE) { - StmtDiff initializerDiff; - if (CNE->hasInitializer()) - initializerDiff = Visit(CNE->getInitializer(), dfdx()); - - Expr* clonedArraySizeE = nullptr; - Expr* derivedArraySizeE = nullptr; - if (CNE->getArraySize()) { - clonedArraySizeE = - Visit(clad_compat::ArraySize_GetValue(CNE->getArraySize())).getExpr(); - // Array size is a non-differentiable expression, thus the original value - // should be used in both the cloned and the derived statements. - derivedArraySizeE = Clone(clonedArraySizeE); - } - Expr* clonedNewE = utils::BuildCXXNewExpr( - m_Sema, CNE->getAllocatedType(), clonedArraySizeE, - initializerDiff.getExpr(), CNE->getAllocatedTypeSourceInfo()); - Expr* derivedNewE = utils::BuildCXXNewExpr( - m_Sema, CNE->getAllocatedType(), derivedArraySizeE, - initializerDiff.getExpr_dx(), CNE->getAllocatedTypeSourceInfo()); - return {clonedNewE, derivedNewE}; -} + StmtDiff initializerDiff; + if (CNE->hasInitializer()) + initializerDiff = Visit(CNE->getInitializer(), dfdx()); + + Expr* clonedArraySizeE = nullptr; + Expr* derivedArraySizeE = nullptr; + if (CNE->getArraySize()) { + clonedArraySizeE = + Visit(clad_compat::ArraySize_GetValue(CNE->getArraySize())).getExpr(); + // Array size is a non-differentiable expression, thus the original value + // should be used in both the cloned and the derived statements. + derivedArraySizeE = Clone(clonedArraySizeE); + } + Expr* clonedNewE = utils::BuildCXXNewExpr( + m_Sema, CNE->getAllocatedType(), clonedArraySizeE, + initializerDiff.getExpr(), CNE->getAllocatedTypeSourceInfo()); + Expr* derivedNewE = utils::BuildCXXNewExpr( + m_Sema, CNE->getAllocatedType(), derivedArraySizeE, + initializerDiff.getExpr_dx(), CNE->getAllocatedTypeSourceInfo()); + return {clonedNewE, derivedNewE}; + } -StmtDiff -ReverseModeVisitor::VisitCXXDeleteExpr(const clang::CXXDeleteExpr* CDE) { - StmtDiff argDiff = Visit(CDE->getArgument()); - Expr* clonedDeleteE = - m_Sema - .ActOnCXXDelete(noLoc, CDE->isGlobalDelete(), CDE->isArrayForm(), - argDiff.getExpr()) - .get(); - Expr* derivedDeleteE = - m_Sema - .ActOnCXXDelete(noLoc, CDE->isGlobalDelete(), CDE->isArrayForm(), - argDiff.getExpr_dx()) - .get(); - // create a compound statement containing both the cloned and the derived - // delete expressions. - CompoundStmt* CS = MakeCompoundStmt({clonedDeleteE, derivedDeleteE}); - m_DeallocExprs.push_back(CS); - return {nullptr, nullptr}; -} + StmtDiff + ReverseModeVisitor::VisitCXXDeleteExpr(const clang::CXXDeleteExpr* CDE) { + StmtDiff argDiff = Visit(CDE->getArgument()); + Expr* clonedDeleteE = + m_Sema + .ActOnCXXDelete(noLoc, CDE->isGlobalDelete(), CDE->isArrayForm(), + argDiff.getExpr()) + .get(); + Expr* derivedDeleteE = + m_Sema + .ActOnCXXDelete(noLoc, CDE->isGlobalDelete(), CDE->isArrayForm(), + argDiff.getExpr_dx()) + .get(); + // create a compound statement containing both the cloned and the derived + // delete expressions. + CompoundStmt* CS = MakeCompoundStmt({clonedDeleteE, derivedDeleteE}); + m_DeallocExprs.push_back(CS); + return {nullptr, nullptr}; + } // FIXME: Add support for differentiating calls to constructors. // We currently assume that constructor arguments are non-differentiable. diff --git a/test/ForwardMode/Pointer.C b/test/ForwardMode/Pointer.C index de181231d..25fce39b3 100644 --- a/test/ForwardMode/Pointer.C +++ b/test/ForwardMode/Pointer.C @@ -110,16 +110,41 @@ double fn5(double i, double j) { // CHECK-NEXT: return *(_d_arr + idx1) + *(_d_arr + idx2); // CHECK-NEXT: } +struct T { + double i; + int j; +}; + +double fn6 (double i) { + T* t = new T{i}; + double res = t->i; + delete t; + return res; +} + +// CHECK: double fn6_darg0(double i) { +// CHECK-NEXT: double _d_i = 1; +// CHECK-NEXT: T *_d_t = new T({_d_i, /*implicit*/(int)0}); +// CHECK-NEXT: T *t = new T({i, /*implicit*/(int)0}); +// CHECK-NEXT: double _d_res = _d_t->i; +// CHECK-NEXT: double res = t->i; +// CHECK-NEXT: delete _d_t; +// CHECK-NEXT: delete t; +// CHECK-NEXT: return _d_res; +// CHECK-NEXT: } + int main() { INIT_DIFFERENTIATE(fn1, "i"); INIT_DIFFERENTIATE(fn2, "i"); INIT_DIFFERENTIATE(fn3, "i"); INIT_DIFFERENTIATE(fn4, "i"); INIT_DIFFERENTIATE(fn5, "i"); + INIT_DIFFERENTIATE(fn6, "i"); TEST_DIFFERENTIATE(fn1, 3, 5); // CHECK-EXEC: {5.00} TEST_DIFFERENTIATE(fn2, 3, 5); // CHECK-EXEC: {5.00} TEST_DIFFERENTIATE(fn3, 3, 5); // CHECK-EXEC: {6.00} TEST_DIFFERENTIATE(fn4, 3, 5); // CHECK-EXEC: {16.00} TEST_DIFFERENTIATE(fn5, 3, 5); // CHECK-EXEC: {57.00} + TEST_DIFFERENTIATE(fn6, 3); // CHECK-EXEC: {1.00} } diff --git a/test/Gradient/Pointers.C b/test/Gradient/Pointers.C index 11a66705c..cb2b66ee9 100644 --- a/test/Gradient/Pointers.C +++ b/test/Gradient/Pointers.C @@ -395,14 +395,40 @@ double newAndDeletePointer(double i, double j) { // CHECK-NEXT: } // CHECK-NEXT: * _d_j += *_d_q; // CHECK-NEXT: * _d_i += *_d_p; -// CHECK-NEXT: delete [] r; -// CHECK-NEXT: delete [] _d_r; -// CHECK-NEXT: delete q; -// CHECK-NEXT: delete _d_q; // CHECK-NEXT: delete p; // CHECK-NEXT: delete _d_p; +// CHECK-NEXT: delete q; +// CHECK-NEXT: delete _d_q; +// CHECK-NEXT: delete [] r; +// CHECK-NEXT: delete [] _d_r; +// CHECK-NEXT: } + +struct T { + double x; + int y; +}; + +double structPointer (double x) { + T* t = new T{x}; + double res = t->x; + delete t; + return res; +} + +// CHECK: void structPointer_grad(double x, clad::array_ref _d_x) { +// CHECK-NEXT: T *_d_t = 0; +// CHECK-NEXT: double _d_res = 0; +// CHECK-NEXT: _d_t = new T; +// CHECK-NEXT: T *t = new T({x, /*implicit*/(int)0}); +// CHECK-NEXT: double res = t->x; +// CHECK-NEXT: goto _label0; +// CHECK-NEXT: _label0: +// CHECK-NEXT: _d_res += 1; +// CHECK-NEXT: _d_t->x += _d_res; +// CHECK-NEXT: * _d_x += *_d_t.x; +// CHECK-NEXT: delete t; +// CHECK-NEXT: delete _d_t; // CHECK-NEXT: } - #define NON_MEM_FN_TEST(var)\ res[0]=0;\ @@ -503,4 +529,8 @@ int main() { double d_i = 0, d_j = 0; d_newAndDeletePointer.execute(5, 7, &d_i, &d_j); printf("%.2f %.2f\n", d_i, d_j); // CHECK-EXEC: 9.00 7.00 + + auto d_structPointer = clad::gradient(structPointer); + double d_x = 0; + d_structPointer.execute(5, &d_x); } \ No newline at end of file From 228dbc7e76b81f1419b13cb42998fca474844fd1 Mon Sep 17 00:00:00 2001 From: Vaibhav Thakkar Date: Thu, 22 Feb 2024 14:11:41 +0100 Subject: [PATCH 3/6] Add support for CStyleCastExpr --- .../Differentiator/BaseForwardModeVisitor.h | 1 + .../clad/Differentiator/ReverseModeVisitor.h | 1 + lib/Differentiator/BaseForwardModeVisitor.cpp | 19 +++++++++++++++++++ lib/Differentiator/ReverseModeVisitor.cpp | 19 ++++++++++++++++++- 4 files changed, 39 insertions(+), 1 deletion(-) diff --git a/include/clad/Differentiator/BaseForwardModeVisitor.h b/include/clad/Differentiator/BaseForwardModeVisitor.h index 4ca1fcae3..b34de9c8e 100644 --- a/include/clad/Differentiator/BaseForwardModeVisitor.h +++ b/include/clad/Differentiator/BaseForwardModeVisitor.h @@ -107,6 +107,7 @@ class BaseForwardModeVisitor StmtDiff VisitSubstNonTypeTemplateParmExpr( const clang::SubstNonTypeTemplateParmExpr* NTTP); StmtDiff VisitImplicitValueInitExpr(const clang::ImplicitValueInitExpr* IVIE); + StmtDiff VisitCStyleCastExpr(const clang::CStyleCastExpr* CSCE); virtual clang::QualType GetPushForwardDerivativeType(clang::QualType ParamType); diff --git a/include/clad/Differentiator/ReverseModeVisitor.h b/include/clad/Differentiator/ReverseModeVisitor.h index dff250161..10564dbbf 100644 --- a/include/clad/Differentiator/ReverseModeVisitor.h +++ b/include/clad/Differentiator/ReverseModeVisitor.h @@ -360,6 +360,7 @@ namespace clad { StmtDiff VisitImplicitCastExpr(const clang::ImplicitCastExpr* ICE); StmtDiff VisitImplicitValueInitExpr(const clang::ImplicitValueInitExpr* IVIE); + StmtDiff VisitCStyleCastExpr(const clang::CStyleCastExpr* CSCE); StmtDiff VisitInitListExpr(const clang::InitListExpr* ILE); StmtDiff VisitIntegerLiteral(const clang::IntegerLiteral* IL); StmtDiff VisitMemberExpr(const clang::MemberExpr* ME); diff --git a/lib/Differentiator/BaseForwardModeVisitor.cpp b/lib/Differentiator/BaseForwardModeVisitor.cpp index 68979bcb3..520e025c7 100644 --- a/lib/Differentiator/BaseForwardModeVisitor.cpp +++ b/lib/Differentiator/BaseForwardModeVisitor.cpp @@ -1450,6 +1450,25 @@ StmtDiff BaseForwardModeVisitor::VisitImplicitValueInitExpr( return StmtDiff(Clone(E), Clone(E)); } +StmtDiff +BaseForwardModeVisitor::VisitCStyleCastExpr(const CStyleCastExpr* CSCE) { + StmtDiff subExprDiff = Visit(CSCE->getSubExpr()); + // Create a new CStyleCastExpr with the same type and the same subexpression + // as the original one. + Expr* castExpr = m_Sema + .BuildCStyleCastExpr( + CSCE->getLParenLoc(), CSCE->getTypeInfoAsWritten(), + CSCE->getRParenLoc(), subExprDiff.getExpr()) + .get(); + Expr* castExprDiff = + m_Sema + .BuildCStyleCastExpr(CSCE->getLParenLoc(), + CSCE->getTypeInfoAsWritten(), + CSCE->getRParenLoc(), subExprDiff.getExpr_dx()) + .get(); + return StmtDiff(castExpr, castExprDiff); +} + StmtDiff BaseForwardModeVisitor::VisitCXXDefaultArgExpr(const CXXDefaultArgExpr* DE) { // FIXME: Shouldn't we simply clone the CXXDefaultArgExpr? diff --git a/lib/Differentiator/ReverseModeVisitor.cpp b/lib/Differentiator/ReverseModeVisitor.cpp index 8a010d283..b792c29a8 100644 --- a/lib/Differentiator/ReverseModeVisitor.cpp +++ b/lib/Differentiator/ReverseModeVisitor.cpp @@ -1429,7 +1429,7 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, } } if (allArgsAreConstantLiterals) - return StmtDiff(Clone(CE)); + return StmtDiff(Clone(CE), Clone(CE)); } // Stores the call arguments for the function to be derived @@ -2943,6 +2943,23 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, return {Clone(IVIE), Clone(IVIE)}; } + StmtDiff ReverseModeVisitor::VisitCStyleCastExpr(const CStyleCastExpr* CSCE) { + StmtDiff subExprDiff = Visit(CSCE->getSubExpr(), dfdx()); + Expr* castExpr = m_Sema + .BuildCStyleCastExpr( + CSCE->getLParenLoc(), CSCE->getTypeInfoAsWritten(), + CSCE->getRParenLoc(), subExprDiff.getExpr()) + .get(); + Expr* castExprDiff = subExprDiff.getExpr_dx(); + if (castExprDiff != nullptr) + castExprDiff = m_Sema + .BuildCStyleCastExpr( + CSCE->getLParenLoc(), CSCE->getTypeInfoAsWritten(), + CSCE->getRParenLoc(), subExprDiff.getExpr_dx()) + .get(); + return {castExpr, castExprDiff}; + } + StmtDiff ReverseModeVisitor::VisitMemberExpr(const MemberExpr* ME) { auto baseDiff = VisitWithExplicitNoDfDx(ME->getBase()); auto* field = ME->getMemberDecl(); From df50c3a178a0bd016f2554c275671590050f738a Mon Sep 17 00:00:00 2001 From: Vaibhav Thakkar Date: Thu, 22 Feb 2024 15:42:11 +0100 Subject: [PATCH 4/6] Add support for C-style memory alloc and free in reverse mode AD --- include/clad/Differentiator/CladUtils.h | 3 ++ lib/Differentiator/CladUtils.cpp | 14 ++++++ lib/Differentiator/ReverseModeVisitor.cpp | 52 +++++++++++++++++++++++ test/Gradient/Pointers.C | 38 +++++++++++++++++ 4 files changed, 107 insertions(+) diff --git a/include/clad/Differentiator/CladUtils.h b/include/clad/Differentiator/CladUtils.h index 5690c3913..9b086ac50 100644 --- a/include/clad/Differentiator/CladUtils.h +++ b/include/clad/Differentiator/CladUtils.h @@ -328,6 +328,9 @@ namespace clad { void SetSwitchCaseSubStmt(clang::SwitchCase* SC, clang::Stmt* subStmt); bool IsLiteral(const clang::Expr* E); + + bool IsMemoryAllocationFunction(const clang::FunctionDecl* FD); + bool IsMemoryDeallocationFunction(const clang::FunctionDecl* FD); } // namespace utils } // namespace clad diff --git a/lib/Differentiator/CladUtils.cpp b/lib/Differentiator/CladUtils.cpp index fbddd535b..3d1a872c9 100644 --- a/lib/Differentiator/CladUtils.cpp +++ b/lib/Differentiator/CladUtils.cpp @@ -641,5 +641,19 @@ namespace clad { isa(E) || isa(E) || isa(E); } + + bool IsMemoryAllocationFunction(const clang::FunctionDecl* FD) { + if (FD->getBuiltinID() == Builtin::BImalloc) + return true; + if (FD->getBuiltinID() == Builtin::BIcalloc) + return true; + if (FD->getBuiltinID() == Builtin::BIrealloc) + return true; + return false; + } + + bool IsMemoryDeallocationFunction(const clang::FunctionDecl* FD) { + return FD->getBuiltinID() == Builtin::BIfree; + } } // namespace utils } // namespace clad diff --git a/lib/Differentiator/ReverseModeVisitor.cpp b/lib/Differentiator/ReverseModeVisitor.cpp index b792c29a8..c0acb6f0f 100644 --- a/lib/Differentiator/ReverseModeVisitor.cpp +++ b/lib/Differentiator/ReverseModeVisitor.cpp @@ -1441,6 +1441,58 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, // Stores tape decl and pushes for multiarg numerically differentiated // calls. llvm::SmallVector NumericalDiffMultiArg{}; + + // For calls to C-style memory allocation functions, we do not need to + // differentiate the call. We just need to visit the arguments to the + // function. + if (utils::IsMemoryAllocationFunction(FD)) { + for (const Expr* Arg : CE->arguments()) { + StmtDiff ArgDiff = Visit(Arg, dfdx()); + CallArgs.push_back(ArgDiff.getExpr()); + if (Arg->getType()->isPointerType()) + DerivedCallArgs.push_back(ArgDiff.getExpr_dx()); + else + DerivedCallArgs.push_back(ArgDiff.getExpr()); + } + Expr* call = + m_Sema + .ActOnCallExpr(getCurrentScope(), Clone(CE->getCallee()), noLoc, + llvm::MutableArrayRef(CallArgs), noLoc) + .get(); + Expr* call_dx = + m_Sema + .ActOnCallExpr(getCurrentScope(), Clone(CE->getCallee()), noLoc, + llvm::MutableArrayRef(DerivedCallArgs), + noLoc) + .get(); + return StmtDiff(call, call_dx); + } + // For calls to C-style memory deallocation functions, we do not need to + // differentiate the call. We just need to visit the arguments to the + // function. Also, don't add any statements either in forward or reverse + // pass. Instead, add it in m_DeallocExprs. + if (utils::IsMemoryDeallocationFunction(FD)) { + for (const Expr* Arg : CE->arguments()) { + StmtDiff ArgDiff = Visit(Arg, dfdx()); + CallArgs.push_back(ArgDiff.getExpr()); + DerivedCallArgs.push_back(ArgDiff.getExpr_dx()); + } + Expr* call = + m_Sema + .ActOnCallExpr(getCurrentScope(), Clone(CE->getCallee()), noLoc, + llvm::MutableArrayRef(CallArgs), noLoc) + .get(); + Expr* call_dx = + m_Sema + .ActOnCallExpr(getCurrentScope(), Clone(CE->getCallee()), noLoc, + llvm::MutableArrayRef(DerivedCallArgs), + noLoc) + .get(); + m_DeallocExprs.push_back(call); + m_DeallocExprs.push_back(call_dx); + return StmtDiff(); + } + // If the result does not depend on the result of the call, just clone // the call and visit arguments (since they may contain side-effects like // f(x = y)) diff --git a/test/Gradient/Pointers.C b/test/Gradient/Pointers.C index cb2b66ee9..f6f1c78ba 100644 --- a/test/Gradient/Pointers.C +++ b/test/Gradient/Pointers.C @@ -430,6 +430,38 @@ double structPointer (double x) { // CHECK-NEXT: delete _d_t; // CHECK-NEXT: } +double cStyleMemoryAlloc(double x, size_t n) { + T* t = (T*)malloc(n * sizeof(T)); + t->x = x; + double res = t->x; + free(t); + return res; +} + +// CHECK: void cStyleMemoryAlloc_grad_0(double x, size_t n, clad::array_ref _d_x) { +// CHECK-NEXT: size_t _d_n = 0; +// CHECK-NEXT: T *_d_t = 0; +// CHECK-NEXT: double _t0; +// CHECK-NEXT: double _d_res = 0; +// CHECK-NEXT: _d_t = (T *)malloc(n * sizeof(T)); +// CHECK-NEXT: T *t = (T *)malloc(n * sizeof(T)); +// CHECK-NEXT: _t0 = t->x; +// CHECK-NEXT: t->x = x; +// CHECK-NEXT: double res = t->x; +// CHECK-NEXT: goto _label0; +// CHECK-NEXT: _label0: +// CHECK-NEXT: _d_res += 1; +// CHECK-NEXT: _d_t->x += _d_res; +// CHECK-NEXT: { +// CHECK-NEXT: t->x = _t0; +// CHECK-NEXT: double _r_d0 = _d_t->x; +// CHECK-NEXT: _d_t->x -= _r_d0; +// CHECK-NEXT: * _d_x += _r_d0; +// CHECK-NEXT: } +// CHECK-NEXT: free(t); +// CHECK-NEXT: free(_d_t); +// CHECK-NEXT: } + #define NON_MEM_FN_TEST(var)\ res[0]=0;\ var.execute(5,res);\ @@ -533,4 +565,10 @@ int main() { auto d_structPointer = clad::gradient(structPointer); double d_x = 0; d_structPointer.execute(5, &d_x); + printf("%.2f\n", d_x); // CHECK-EXEC: 1.00 + + auto d_cStyleMemoryAlloc = clad::gradient(cStyleMemoryAlloc, "x"); + d_x = 0; + d_cStyleMemoryAlloc.execute(5, 7, &d_x); + printf("%.2f\n", d_x); // CHECK-EXEC: 1.00 } \ No newline at end of file From e37039d58d289e9e43c2222f4ad18de8bd7faaa1 Mon Sep 17 00:00:00 2001 From: Vaibhav Thakkar Date: Thu, 22 Feb 2024 16:37:53 +0100 Subject: [PATCH 5/6] Add support for C-style memory alloc and free in forward mode AD --- .../clad/Differentiator/BuiltinDerivatives.h | 25 ++++++++ test/ForwardMode/Pointer.C | 43 ++++++++++++++ test/Gradient/Pointers.C | 58 +++++++++++++++++-- 3 files changed, 122 insertions(+), 4 deletions(-) diff --git a/include/clad/Differentiator/BuiltinDerivatives.h b/include/clad/Differentiator/BuiltinDerivatives.h index fac077f00..b2c5875d8 100644 --- a/include/clad/Differentiator/BuiltinDerivatives.h +++ b/include/clad/Differentiator/BuiltinDerivatives.h @@ -196,6 +196,31 @@ CUDA_HOST_DEVICE void clamp_pullback(const T& v, const T& lo, const T& hi, #endif } // namespace std + +// NOLINTBEGIN(cppcoreguidelines-no-malloc) +// NOLINTBEGIN(cppcoreguidelines-owning-memory) +inline ValueAndPushforward malloc_pushforward(size_t sz, + size_t d_sz) { + return {malloc(sz), malloc(sz)}; +} + +inline ValueAndPushforward +calloc_pushforward(size_t n, size_t sz, size_t d_n, size_t d_sz) { + return {calloc(n, sz), calloc(n, sz)}; +} + +inline ValueAndPushforward +realloc_pushforward(void* ptr, size_t sz, void* d_ptr, size_t d_sz) { + return {realloc(ptr, sz), realloc(d_ptr, sz)}; +} + +inline void free_pushforward(void* ptr, void* d_ptr) { + free(ptr); + free(d_ptr); +} +// NOLINTEND(cppcoreguidelines-owning-memory) +// NOLINTEND(cppcoreguidelines-no-malloc) + // These are required because C variants of mathematical functions are // defined in global namespace. using std::abs_pushforward; diff --git a/test/ForwardMode/Pointer.C b/test/ForwardMode/Pointer.C index 25fce39b3..5190f5cc5 100644 --- a/test/ForwardMode/Pointer.C +++ b/test/ForwardMode/Pointer.C @@ -133,6 +133,47 @@ double fn6 (double i) { // CHECK-NEXT: return _d_res; // CHECK-NEXT: } +double fn7(double i) { + double *p = (double*)malloc(8UL /*sizeof(double)*/); + *p = i; + T *t = (T*)calloc(1, sizeof(T)); + t->i = i; + double res = *p + t->i; + p = (double*)realloc(p, 2*sizeof(double)); + p[1] = 2*i; + res += p[1]; + free(t); + free(p); + return res; +} + +// CHECK: double fn7_darg0(double i) { +// CHECK-NEXT: double _d_i = 1; +// CHECK-NEXT: {{(clad::)?}}ValueAndPushforward _t0 = clad::custom_derivatives::malloc_pushforward(8UL, 0UL); +// CHECK-NEXT: double *_d_p = (double *)_t0.pushforward; +// CHECK-NEXT: double *p = (double *)_t0.value; +// CHECK-NEXT: *_d_p = _d_i; +// CHECK-NEXT: *p = i; +// CHECK-NEXT: {{(clad::)?}}ValueAndPushforward _t1 = clad::custom_derivatives::calloc_pushforward(1, sizeof(T), 0, sizeof(T)); +// CHECK-NEXT: T *_d_t = (T *)_t1.pushforward; +// CHECK-NEXT: T *t = (T *)_t1.value; +// CHECK-NEXT: _d_t->i = _d_i; +// CHECK-NEXT: t->i = i; +// CHECK-NEXT: double _d_res = *_d_p + _d_t->i; +// CHECK-NEXT: double res = *p + t->i; +// CHECK-NEXT: unsigned long _t2 = sizeof(double); +// CHECK-NEXT: {{(clad::)?}}ValueAndPushforward _t3 = clad::custom_derivatives::realloc_pushforward(p, 2 * _t2, _d_p, 0 * _t2 + 2 * sizeof(double)); +// CHECK-NEXT: _d_p = (double *)_t3.pushforward; +// CHECK-NEXT: p = (double *)_t3.value; +// CHECK-NEXT: _d_p[1] = 0 * i + 2 * _d_i; +// CHECK-NEXT: p[1] = 2 * i; +// CHECK-NEXT: _d_res += _d_p[1]; +// CHECK-NEXT: res += p[1]; +// CHECK-NEXT: clad::custom_derivatives::free_pushforward(t, _d_t); +// CHECK-NEXT: clad::custom_derivatives::free_pushforward(p, _d_p); +// CHECK-NEXT: return _d_res; +// CHECK-NEXT: } + int main() { INIT_DIFFERENTIATE(fn1, "i"); INIT_DIFFERENTIATE(fn2, "i"); @@ -140,6 +181,7 @@ int main() { INIT_DIFFERENTIATE(fn4, "i"); INIT_DIFFERENTIATE(fn5, "i"); INIT_DIFFERENTIATE(fn6, "i"); + INIT_DIFFERENTIATE(fn7, "i"); TEST_DIFFERENTIATE(fn1, 3, 5); // CHECK-EXEC: {5.00} TEST_DIFFERENTIATE(fn2, 3, 5); // CHECK-EXEC: {5.00} @@ -147,4 +189,5 @@ int main() { TEST_DIFFERENTIATE(fn4, 3, 5); // CHECK-EXEC: {16.00} TEST_DIFFERENTIATE(fn5, 3, 5); // CHECK-EXEC: {57.00} TEST_DIFFERENTIATE(fn6, 3); // CHECK-EXEC: {1.00} + TEST_DIFFERENTIATE(fn7, 3); // CHECK-EXEC: {4.00} } diff --git a/test/Gradient/Pointers.C b/test/Gradient/Pointers.C index f6f1c78ba..c216aba6e 100644 --- a/test/Gradient/Pointers.C +++ b/test/Gradient/Pointers.C @@ -433,7 +433,13 @@ double structPointer (double x) { double cStyleMemoryAlloc(double x, size_t n) { T* t = (T*)malloc(n * sizeof(T)); t->x = x; - double res = t->x; + double* p = (double*)calloc(1, sizeof(double)); + *p = x; + double res = t->x + *p; + p = (double*)realloc(p, 2*sizeof(double)); + p[1] = 2*x; + res += p[1]; + free(p); free(t); return res; } @@ -442,22 +448,66 @@ double cStyleMemoryAlloc(double x, size_t n) { // CHECK-NEXT: size_t _d_n = 0; // CHECK-NEXT: T *_d_t = 0; // CHECK-NEXT: double _t0; +// CHECK-NEXT: double *_d_p = 0; +// CHECK-NEXT: double _t1; // CHECK-NEXT: double _d_res = 0; +// CHECK-NEXT: double *_t2; +// CHECK-NEXT: double *_t3; +// CHECK-NEXT: double _t4; +// CHECK-NEXT: double _t5; // CHECK-NEXT: _d_t = (T *)malloc(n * sizeof(T)); // CHECK-NEXT: T *t = (T *)malloc(n * sizeof(T)); // CHECK-NEXT: _t0 = t->x; // CHECK-NEXT: t->x = x; -// CHECK-NEXT: double res = t->x; +// CHECK-NEXT: _d_p = (double *)calloc(1, sizeof(double)); +// CHECK-NEXT: double *p = (double *)calloc(1, sizeof(double)); +// CHECK-NEXT: _t1 = *p; +// CHECK-NEXT: *p = x; +// CHECK-NEXT: double res = t->x + *p; +// CHECK-NEXT: _t2 = p; +// CHECK-NEXT: _t3 = _d_p; +// CHECK-NEXT: _d_p = (double *)realloc(_d_p, 2 * sizeof(double)); +// CHECK-NEXT: p = (double *)realloc(p, 2 * sizeof(double)); +// CHECK-NEXT: _t4 = p[1]; +// CHECK-NEXT: p[1] = 2 * x; +// CHECK-NEXT: _t5 = res; +// CHECK-NEXT: res += p[1]; // CHECK-NEXT: goto _label0; // CHECK-NEXT: _label0: // CHECK-NEXT: _d_res += 1; -// CHECK-NEXT: _d_t->x += _d_res; +// CHECK-NEXT: { +// CHECK-NEXT: res = _t5; +// CHECK-NEXT: double _r_d3 = _d_res; +// CHECK-NEXT: _d_p[1] += _r_d3; +// CHECK-NEXT: } +// CHECK-NEXT: { +// CHECK-NEXT: p[1] = _t4; +// CHECK-NEXT: double _r_d2 = _d_p[1]; +// CHECK-NEXT: _d_p[1] -= _r_d2; +// CHECK-NEXT: * _d_x += 2 * _r_d2; +// CHECK-NEXT: } +// CHECK-NEXT: { +// CHECK-NEXT: p = _t2; +// CHECK-NEXT: _d_p = _t3; +// CHECK-NEXT: } +// CHECK-NEXT: { +// CHECK-NEXT: _d_t->x += _d_res; +// CHECK-NEXT: *_d_p += _d_res; +// CHECK-NEXT: } +// CHECK-NEXT: { +// CHECK-NEXT: *p = _t1; +// CHECK-NEXT: double _r_d1 = *_d_p; +// CHECK-NEXT: *_d_p -= _r_d1; +// CHECK-NEXT: * _d_x += _r_d1; +// CHECK-NEXT: } // CHECK-NEXT: { // CHECK-NEXT: t->x = _t0; // CHECK-NEXT: double _r_d0 = _d_t->x; // CHECK-NEXT: _d_t->x -= _r_d0; // CHECK-NEXT: * _d_x += _r_d0; // CHECK-NEXT: } +// CHECK-NEXT: free(p); +// CHECK-NEXT: free(_d_p); // CHECK-NEXT: free(t); // CHECK-NEXT: free(_d_t); // CHECK-NEXT: } @@ -570,5 +620,5 @@ int main() { auto d_cStyleMemoryAlloc = clad::gradient(cStyleMemoryAlloc, "x"); d_x = 0; d_cStyleMemoryAlloc.execute(5, 7, &d_x); - printf("%.2f\n", d_x); // CHECK-EXEC: 1.00 + printf("%.2f\n", d_x); // CHECK-EXEC: 4.00 } \ No newline at end of file From e1cf160fe71a16ee9077d2962985f68a7c6a1dab Mon Sep 17 00:00:00 2001 From: Vaibhav Thakkar Date: Mon, 26 Feb 2024 12:41:46 +0100 Subject: [PATCH 6/6] Fix builtin macros and zero init for memory operations --- .clang-tidy | 1 + include/clad/Differentiator/CladUtils.h | 2 +- include/clad/Differentiator/Compatibility.h | 11 +++++++ lib/Differentiator/CladUtils.cpp | 33 ++++++++++++++++++--- lib/Differentiator/ReverseModeVisitor.cpp | 29 ++++++++++++++---- test/ForwardMode/Pointer.C | 2 +- test/Gradient/Pointers.C | 7 +++-- 7 files changed, 71 insertions(+), 14 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 9c90276eb..894a98c74 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -28,6 +28,7 @@ Checks: > -readability-function-cognitive-complexity, -readability-implicit-bool-conversion, -cppcoreguidelines-avoid-magic-numbers, + -clang-analyzer-cplusplus.NewDeleteLeaks, CheckOptions: - key: readability-identifier-naming.ClassCase diff --git a/include/clad/Differentiator/CladUtils.h b/include/clad/Differentiator/CladUtils.h index 9b086ac50..56519593d 100644 --- a/include/clad/Differentiator/CladUtils.h +++ b/include/clad/Differentiator/CladUtils.h @@ -329,7 +329,7 @@ namespace clad { bool IsLiteral(const clang::Expr* E); - bool IsMemoryAllocationFunction(const clang::FunctionDecl* FD); + bool IsMemoryFunction(const clang::FunctionDecl* FD); bool IsMemoryDeallocationFunction(const clang::FunctionDecl* FD); } // namespace utils } // namespace clad diff --git a/include/clad/Differentiator/Compatibility.h b/include/clad/Differentiator/Compatibility.h index 62d71641c..9f11a652e 100644 --- a/include/clad/Differentiator/Compatibility.h +++ b/include/clad/Differentiator/Compatibility.h @@ -766,5 +766,16 @@ static inline const DeclSpec& Sema_ActOnStartOfLambdaDefinition_ScopeOrDeclSpec( ,Node->isTransparent() #endif +// Clang 9 above added isa_and_nonnull. +#if CLANG_VERSION_MAJOR < 9 +template bool isa_and_nonnull(const Y* Val) { + return Val && isa(Val); +} +#else +template bool isa_and_nonnull(const Y* Val) { + return llvm::isa_and_nonnull(Val); +} +#endif + } // namespace clad_compat #endif //CLAD_COMPATIBILITY diff --git a/lib/Differentiator/CladUtils.cpp b/lib/Differentiator/CladUtils.cpp index 3d1a872c9..d7d599ce5 100644 --- a/lib/Differentiator/CladUtils.cpp +++ b/lib/Differentiator/CladUtils.cpp @@ -6,6 +6,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Basic/Builtins.h" #include "clang/Basic/SourceLocation.h" #include "clang/Sema/Lookup.h" #include "llvm/ADT/SmallVector.h" @@ -400,6 +401,12 @@ namespace clad { auto& C = semaRef.getASTContext(); if (!TSI) TSI = C.getTrivialTypeSourceInfo(qType); + if (clad_compat::isa_and_nonnull(initializer)) + // If the initializer is an implicit value init expression, then + // we don't need to pass it explicitly to the CXXNewExpr. As, clang + // internally adds it when initializer is ParenListExpr and + // DirectInitRange is valid. + initializer = semaRef.ActOnParenListExpr(noLoc, noLoc, {}).get(); auto newExpr = semaRef .BuildCXXNew( @@ -642,18 +649,36 @@ namespace clad { isa(E); } - bool IsMemoryAllocationFunction(const clang::FunctionDecl* FD) { + bool IsMemoryFunction(const clang::FunctionDecl* FD) { + +#if CLANG_VERSION_MAJOR > 12 if (FD->getBuiltinID() == Builtin::BImalloc) return true; - if (FD->getBuiltinID() == Builtin::BIcalloc) + if (FD->getBuiltinID() == Builtin::ID::BIcalloc) + return true; + if (FD->getBuiltinID() == Builtin::ID::BIrealloc) + return true; + if (FD->getBuiltinID() == Builtin::ID::BImemset) + return true; +#else + if (FD->getNameAsString() == "malloc") return true; - if (FD->getBuiltinID() == Builtin::BIrealloc) + if (FD->getNameAsString() == "calloc") return true; + if (FD->getNameAsString() == "realloc") + return true; + if (FD->getNameAsString() == "memset") + return true; +#endif return false; } bool IsMemoryDeallocationFunction(const clang::FunctionDecl* FD) { - return FD->getBuiltinID() == Builtin::BIfree; +#if CLANG_VERSION_MAJOR > 12 + return FD->getBuiltinID() == Builtin::ID::BIfree; +#else + return FD->getNameAsString() == "free"; +#endif } } // namespace utils } // namespace clad diff --git a/lib/Differentiator/ReverseModeVisitor.cpp b/lib/Differentiator/ReverseModeVisitor.cpp index c0acb6f0f..a181785ca 100644 --- a/lib/Differentiator/ReverseModeVisitor.cpp +++ b/lib/Differentiator/ReverseModeVisitor.cpp @@ -1445,7 +1445,7 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, // For calls to C-style memory allocation functions, we do not need to // differentiate the call. We just need to visit the arguments to the // function. - if (utils::IsMemoryAllocationFunction(FD)) { + if (utils::IsMemoryFunction(FD)) { for (const Expr* Arg : CE->arguments()) { StmtDiff ArgDiff = Visit(Arg, dfdx()); CallArgs.push_back(ArgDiff.getExpr()); @@ -2649,8 +2649,7 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, bool isPointerType = VD->getType()->isPointerType(); bool isInitializedByNewExpr = false; // Check if the variable is pointer type and initialized by new expression - if (isPointerType && (VD->getInit() != nullptr) && - isa(VD->getInit())) + if (isPointerType && VD->getInit() && isa(VD->getInit())) isInitializedByNewExpr = true; // VDDerivedInit now serves two purposes -- as the initial derivative value @@ -2842,7 +2841,20 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, if (m_ExternalSource) m_ExternalSource->ActBeforeFinalizingDifferentiateSingleStmt(direction::reverse); - addToCurrentBlock(SDiff.getStmt_dx(), direction::reverse); + // If the statement is a standalone call to a memory function, we want to + // add its derived statement in the same block as the original statement. + // For ex: memset(x, 0, 10) -> memset(_d_x, 0, 10) + Stmt* stmtDx = SDiff.getStmt_dx(); + bool dxInForward = false; + if (auto* callExpr = dyn_cast_or_null(stmtDx)) + if (auto* FD = dyn_cast(callExpr->getCalleeDecl())) + if (utils::IsMemoryFunction(FD)) + dxInForward = true; + + if (dxInForward) + addToCurrentBlock(stmtDx, direction::forward); + else + addToCurrentBlock(SDiff.getStmt_dx(), direction::reverse); CompoundStmt* RCS = endBlock(direction::reverse); std::reverse(RCS->body_begin(), RCS->body_end()); Stmt* ReverseResult = unwrapIfSingleStmt(RCS); @@ -3824,9 +3836,14 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, Expr* clonedNewE = utils::BuildCXXNewExpr( m_Sema, CNE->getAllocatedType(), clonedArraySizeE, initializerDiff.getExpr(), CNE->getAllocatedTypeSourceInfo()); + Expr* diffInit = initializerDiff.getExpr_dx(); + if (!diffInit) { + // we should initialize it implicitly using ParenListExpr. + diffInit = m_Sema.ActOnParenListExpr(noLoc, noLoc, {}).get(); + } Expr* derivedNewE = utils::BuildCXXNewExpr( - m_Sema, CNE->getAllocatedType(), derivedArraySizeE, - initializerDiff.getExpr_dx(), CNE->getAllocatedTypeSourceInfo()); + m_Sema, CNE->getAllocatedType(), derivedArraySizeE, diffInit, + CNE->getAllocatedTypeSourceInfo()); return {clonedNewE, derivedNewE}; } diff --git a/test/ForwardMode/Pointer.C b/test/ForwardMode/Pointer.C index 5190f5cc5..bb1edfdde 100644 --- a/test/ForwardMode/Pointer.C +++ b/test/ForwardMode/Pointer.C @@ -161,7 +161,7 @@ double fn7(double i) { // CHECK-NEXT: t->i = i; // CHECK-NEXT: double _d_res = *_d_p + _d_t->i; // CHECK-NEXT: double res = *p + t->i; -// CHECK-NEXT: unsigned long _t2 = sizeof(double); +// CHECK-NEXT: unsigned {{(int|long)}} _t2 = sizeof(double); // CHECK-NEXT: {{(clad::)?}}ValueAndPushforward _t3 = clad::custom_derivatives::realloc_pushforward(p, 2 * _t2, _d_p, 0 * _t2 + 2 * sizeof(double)); // CHECK-NEXT: _d_p = (double *)_t3.pushforward; // CHECK-NEXT: p = (double *)_t3.value; diff --git a/test/Gradient/Pointers.C b/test/Gradient/Pointers.C index c216aba6e..fe3f0e58a 100644 --- a/test/Gradient/Pointers.C +++ b/test/Gradient/Pointers.C @@ -363,7 +363,7 @@ double newAndDeletePointer(double i, double j) { // CHECK-NEXT: double *p = new double(i); // CHECK-NEXT: _d_q = new double(* _d_j); // CHECK-NEXT: double *q = new double(j); -// CHECK-NEXT: _d_r = new double [2]; +// CHECK-NEXT: _d_r = new double [2](/*implicit*/(double{{[ ]?}}[2])0); // CHECK-NEXT: double *r = new double [2]; // CHECK-NEXT: _t0 = r[0]; // CHECK-NEXT: r[0] = i + j; @@ -418,7 +418,7 @@ double structPointer (double x) { // CHECK: void structPointer_grad(double x, clad::array_ref _d_x) { // CHECK-NEXT: T *_d_t = 0; // CHECK-NEXT: double _d_res = 0; -// CHECK-NEXT: _d_t = new T; +// CHECK-NEXT: _d_t = new T(); // CHECK-NEXT: T *t = new T({x, /*implicit*/(int)0}); // CHECK-NEXT: double res = t->x; // CHECK-NEXT: goto _label0; @@ -432,6 +432,7 @@ double structPointer (double x) { double cStyleMemoryAlloc(double x, size_t n) { T* t = (T*)malloc(n * sizeof(T)); + memset(t, 0, n * sizeof(T)); t->x = x; double* p = (double*)calloc(1, sizeof(double)); *p = x; @@ -457,6 +458,8 @@ double cStyleMemoryAlloc(double x, size_t n) { // CHECK-NEXT: double _t5; // CHECK-NEXT: _d_t = (T *)malloc(n * sizeof(T)); // CHECK-NEXT: T *t = (T *)malloc(n * sizeof(T)); +// CHECK-NEXT: memset(_d_t, 0, n * sizeof(T)); +// CHECK-NEXT: memset(t, 0, n * sizeof(T)); // CHECK-NEXT: _t0 = t->x; // CHECK-NEXT: t->x = x; // CHECK-NEXT: _d_p = (double *)calloc(1, sizeof(double));