Skip to content

Commit 7514327

Browse files
committed
Encode reverse-mode early returns with a named lambda
The reverse-mode early-return encoding emitted a label inside the master reverse and a goto into it from each early-return point. That violates [stmt.dcl]/2 whenever a local with a non-trivial initializer or destructor sits between the goto and its target, which is why DifferentiateWithClad had to bypass Sema::ActOnFinishFunctionBody on the generated function. Materialise the master reverse as a `[&]` lambda bound to an `auto` VarDecl instead, called at each early return and at the tail. The body is assembled outside the closure scope, so its references to captured variables are rebuilt in scope where the `[&]` default registers the capture; the reverse statements are emitted into the closure directly. An early return is a NullStmt marker in the forward block, patched with `{ _rev(); return; }` at finalization so no subtree gains a second parent. Whether a function has a non-tail return is a fact about the primal, so DiffRequest answers hasEarlyReturns() off the primal body; the encoder reads it to pick the lambda vs plain shape up front. A branch condition read by the master reverse is zero-initialized so an early return that skips its forward store leaves it false -- the same zero-initialized additive state clad already relies on for adjoints and loop counters. A return inside a switch case is an early return too: it closes that case's fall-through group so the reverse switch -- rebuilt on the stored condition -- enters only that case's adjoint. The synthesized closure makes Clang's own exception-handling codegen (EHScopeStack landing pad) read an uninitialized value under Valgrind on some runtimes; the read is inside libclang, not clad, and the derivative is correct, so EarlyReturns.C is XFAILed under Valgrind like the other switch/loop tests. Fixes #367
1 parent 59b4e4c commit 7514327

18 files changed

Lines changed: 1070 additions & 368 deletions

include/clad/Differentiator/DiffPlanner.h

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "clang/AST/DeclBase.h"
1212
#include "clang/AST/ExprCXX.h"
1313
#include "clang/AST/RecursiveASTVisitor.h"
14+
#include "clang/AST/Stmt.h"
1415
#include "clang/Analysis/AnalysisDeclContext.h"
1516
#include "clang/Basic/SourceLocation.h"
1617

@@ -42,7 +43,11 @@ using OwnedAnalysisContexts =
4243
llvm::SmallVector<std::unique_ptr<clang::AnalysisDeclContext>, 4>;
4344
using ParamSet = std::set<const clang::ParmVarDecl*>;
4445
using ParamInfo = std::map<const clang::FunctionDecl*, ParamSet>;
45-
/// A struct containing information about request to differentiate a function.
46+
/// A read-only, AD-oriented view over the primal being differentiated: it
47+
/// wraps the primal FunctionDecl and surfaces the AD-relevant facts the
48+
/// FunctionDecl itself does not. Recording such facts here, rather than
49+
/// rediscovering them inside a visitor, keeps them available to every visitor
50+
/// and correct after a request is copied and re-pointed at another Function.
4651
struct DiffRequest {
4752
private:
4853
/// Based on To-Be-Recorded analysis performed before differentiation, tells
@@ -66,7 +71,28 @@ struct DiffRequest {
6671
bool HasAnalysisRun = false;
6772
} m_UsefulRunInfo;
6873

74+
/// Cache for hasEarlyReturns(): whether the primal body has a return that is
75+
/// not in tail position. A property of the Function, computed once on demand.
76+
mutable struct EarlyReturnInfo {
77+
bool HasEarlyReturns = false;
78+
bool HasAnalysisRun = false;
79+
} m_EarlyReturnInfo;
80+
6981
public:
82+
/// The primal body's tail-position return -- the one an early-return encoder
83+
/// lets control fall through to, as opposed to an early return that needs a
84+
/// jump/call. Null when the body does not end in a return. O(1): reads
85+
/// Function's body directly, so a copied request re-pointed at a new Function
86+
/// (a lambda's operator(), a pullback callee) answers for its own Function.
87+
const clang::ReturnStmt* getTailReturn() const;
88+
89+
/// Whether the primal body has a return that is not the tail return -- one
90+
/// the reverse mode must encode with the early-return lambda. A fact about
91+
/// the primal, read directly off Function's body (returns inside nested
92+
/// lambdas belong to their own function and are skipped). A void function
93+
/// seeds no return value, so its returns skip the encoding.
94+
bool hasEarlyReturns() const;
95+
7096
/// Function to be differentiated.
7197
const clang::FunctionDecl* Function = nullptr;
7298
/// Name of the base function to be differentiated. Can be different from
@@ -243,17 +269,17 @@ struct DiffRequest {
243269
bool HasTbrAnalysisRun() const { return m_TbrRunInfo.HasAnalysisRun; }
244270
};
245271

246-
using DiffInterval = std::vector<clang::SourceRange>;
272+
using DiffInterval = std::vector<clang::SourceRange>;
247273

248-
// FIXME: These are translation-unit-wide defaults taken from the compiler
249-
// invocation, not the options of a request; rename to InvocationOptions.
250-
struct RequestOptions {
251-
/// This is a flag to indicate the default behaviour to enable/disable
252-
/// TBR analysis during reverse-mode differentiation.
253-
bool EnableTBRAnalysis = false;
254-
bool EnableVariedAnalysis = false;
255-
bool EnableUsefulAnalysis = false;
256-
};
274+
// FIXME: These are translation-unit-wide defaults taken from the compiler
275+
// invocation, not the options of a request; rename to InvocationOptions.
276+
struct RequestOptions {
277+
/// This is a flag to indicate the default behaviour to enable/disable
278+
/// TBR analysis during reverse-mode differentiation.
279+
bool EnableTBRAnalysis = false;
280+
bool EnableVariedAnalysis = false;
281+
bool EnableUsefulAnalysis = false;
282+
};
257283

258284
class DiffCollector: public clang::RecursiveASTVisitor<DiffCollector> {
259285
/// The source interval where clad was activated.
@@ -298,6 +324,10 @@ struct DiffRequest {
298324
/// or constructor initializers. If we use Visit they would be processed
299325
/// under the parent DiffRequest which is not in the lambda scope.
300326
bool TraverseLambdaExpr(clang::LambdaExpr* LE);
327+
/// Plan a lazily-scheduled nested request the static TU walk never reaches
328+
/// (built in DerivativeBuilder::HandleNestedDiffRequest). Currently records
329+
/// its early-return flag by walking the request's own body.
330+
bool PlanNestedRequest(DiffRequest& request);
301331
bool TraverseFunctionDeclOnce(const clang::FunctionDecl* FD) {
302332
llvm::SaveAndRestore<bool> Saved(m_IsTraversingTopLevelDecl, false);
303333
if (m_Traversed.count(FD))

include/clad/Differentiator/DiffScheduler.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ class DiffScheduler {
3838

3939
/// Static planning pass over a group of top-level declarations.
4040
void Plan(clang::DeclGroupRef DGR) { m_Collector.Walk(DGR); }
41+
42+
/// Plan a single lazily-scheduled request (a nested or higher-order
43+
/// derivative) that the static walk never reached.
44+
void Plan(DiffRequest& R) { m_Collector.PlanNestedRequest(R); }
4145
};
4246

4347
} // namespace clad

include/clad/Differentiator/ReverseModeVisitor.h

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
#include "clang/Sema/Sema.h"
3030
#include "llvm/ADT/ArrayRef.h"
3131
#include "llvm/ADT/DenseSet.h"
32+
#include "llvm/ADT/STLExtras.h" // IWYU pragma: keep -- function_ref on LLVM<14
33+
#include "llvm/ADT/SmallPtrSet.h"
3234
#include "llvm/ADT/SmallVector.h"
3335

3436
#include <array>
@@ -75,6 +77,10 @@ namespace clad {
7577
/// the reverse mode we also accumulate Stmts for the reverse pass which
7678
/// will be executed on return.
7779
std::vector<Stmts> m_Reverse;
80+
/// Markers emitted at early-return sites in the forward block, patched
81+
/// with `{ _rev(); return; }` at finalization, where `_rev` is the lambda
82+
/// wrapping the master reverse sweep.
83+
llvm::SmallPtrSet<clang::Stmt*, 4> m_EarlyReturnMarkers;
7884
/// Storing expressions to delete/free memory in the reverse pass.
7985
Stmts m_DeallocExprs;
8086
/// A dfdx seed: eager expression, or a deferred builder materialized on
@@ -129,6 +135,15 @@ namespace clad {
129135
// Function to Differentiate with Enzyme as Backend
130136
void DifferentiateWithEnzyme();
131137

138+
/// Walk the current forward block and replace every Stmt in
139+
/// m_EarlyReturnMarkers with a fresh Stmt built by `MakeReplacement`.
140+
/// Each marker gets its own node — sharing one replacement across sites
141+
/// would give it multiple parents and break the single-parent AST
142+
/// invariant. Called once at function-body finalization, after the
143+
/// forward sweep is fully assembled and before the body is handed to Sema.
144+
void
145+
patchEarlyReturnMarkers(llvm::function_ref<clang::Stmt*()> MakeReplacement);
146+
132147
public:
133148
using direction = rmv::direction;
134149
virtual clang::Expr* dfdx() {
@@ -327,10 +342,10 @@ namespace clad {
327342
/// of the stack (clad::back(S)).
328343
clang::Expr* GlobalStoreAndRef(clang::Expr* E, clang::QualType Type,
329344
llvm::StringRef prefix = "_t",
330-
bool force = false);
345+
bool force = false, bool zeroInit = false);
331346
clang::Expr* GlobalStoreAndRef(clang::Expr* E,
332347
llvm::StringRef prefix = "_t",
333-
bool force = false);
348+
bool force = false, bool zeroInit = false);
334349
virtual StmtDiff StoreAndRestore(clang::Expr* E,
335350
llvm::StringRef prefix = "_t",
336351
bool moveToTape = false);

include/clad/Differentiator/VisitorBase.h

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "clang/AST/StmtVisitor.h"
1919
#include "clang/AST/Type.h"
2020
#include "clang/Basic/Diagnostic.h"
21+
#include "clang/Basic/Lambda.h"
2122
#include "clang/Basic/OperatorKinds.h"
2223
#include "clang/Basic/Specifiers.h"
2324
#include "clang/Sema/DeclSpec.h"
@@ -26,6 +27,7 @@
2627
#include "clang/Sema/Sema.h"
2728

2829
#include "llvm/ADT/ArrayRef.h"
30+
#include "llvm/ADT/SmallPtrSet.h"
2931
#include "llvm/ADT/StringRef.h"
3032
#include "llvm/Support/PrettyStackTrace.h"
3133

@@ -266,23 +268,56 @@ namespace clad {
266268
/// The currently visited statement. Useful for crash pretty-printing.
267269
const clang::Stmt* m_CurVisitedStmt = nullptr;
268270

271+
/// Resolves the captures of a lambda synthesized from a body that was
272+
/// built outside the closure scope, so Sema never saw the uses. collect()
273+
/// finds the body's free variables -- locals and parameters it references
274+
/// but does not itself declare. resolve() re-creates those references in
275+
/// the current scope, where BuildDeclRef under the `[&]` default registers
276+
/// the capture and marks the reference for codegen; it must run with the
277+
/// lambda scope active. contains() exposes the set for a caller that must
278+
/// place the captured decls before the lambda.
279+
class LambdaCaptures {
280+
VisitorBase& m_V;
281+
llvm::SmallPtrSet<clang::VarDecl*, 8> m_Captures;
282+
283+
public:
284+
explicit LambdaCaptures(VisitorBase& V) : m_V(V) {}
285+
void collect(llvm::ArrayRef<clang::Stmt*> Body);
286+
/// A `[&]` capture binds a variable at the lambda's definition point, so
287+
/// every captured decl must precede the lambda. \p Prefix and \p Suffix
288+
/// are the forward block split at the lambda's insertion point; move each
289+
/// captured DeclStmt from Suffix to the end of Prefix when its
290+
/// initializer references only names already live there -- the function's
291+
/// parameters, \p AlreadyLive (decls emitted earlier), and Prefix.
292+
void orderCaptureDecls(llvm::SmallVectorImpl<clang::Stmt*>& Prefix,
293+
llvm::SmallVectorImpl<clang::Stmt*>& Suffix,
294+
llvm::ArrayRef<clang::Stmt*> AlreadyLive);
295+
void resolve(llvm::ArrayRef<clang::Stmt*> Body);
296+
bool contains(clang::VarDecl* VD) const { return m_Captures.count(VD); }
297+
};
298+
269299
/// Build a lambda whose body is produced by `func`. Returns the
270300
/// LambdaExpr without invoking it, so the caller can bind it to a VarDecl
271301
/// and call it from multiple sites. `func` is invoked inside the lambda's
272302
/// scope and block; statements are expected to be added via
273303
/// addToCurrentBlock from func's invocation.
304+
///
305+
/// The lambda uses the `[&]` capture-default; Sema resolves captures from
306+
/// the body's ODR-uses. A pre-built body whose DeclRefExprs were made
307+
/// outside this scope must have those references rebuilt in scope so Sema
308+
/// sees the uses (see LambdaCaptures::resolve).
274309
// FIXME: This will become problematic when we try to support C.
275310
template <typename F>
276311
static clang::Expr* buildLambda(VisitorBase& V, clang::Sema& S,
277-
const clang::Expr* E, F&& func) {
312+
const clang::Stmt* LocSrc, F&& func) {
278313
// FIXME: Here we use some of the things that are used from Parser, it
279314
// seems to be the easiest way to create lambda.
280315
clang::LambdaIntroducer Intro;
281316
Intro.Default = clang::LCD_ByRef;
282317
// FIXME: Using noLoc here results in assert failure. Any other valid
283318
// SourceLocation seems to work fine.
284-
Intro.Range.setBegin(E->getBeginLoc());
285-
Intro.Range.setEnd(E->getEndLoc());
319+
Intro.Range.setBegin(LocSrc->getBeginLoc());
320+
Intro.Range.setEnd(LocSrc->getEndLoc());
286321
clang::AttributeFactory AttrFactory;
287322
const clang::DeclSpec DS(AttrFactory);
288323
clang::Declarator D(
@@ -329,8 +364,8 @@ namespace clad {
329364
/// added by addToCurrentBlock from func invocation.
330365
template <typename F>
331366
static clang::Expr* wrapInLambda(VisitorBase& V, clang::Sema& S,
332-
const clang::Expr* E, F&& func) {
333-
clang::Expr* lambda = buildLambda(V, S, E, std::forward<F>(func));
367+
const clang::Stmt* LocSrc, F&& func) {
368+
clang::Expr* lambda = buildLambda(V, S, LocSrc, std::forward<F>(func));
334369
return S.ActOnCallExpr(V.getCurrentScope(), lambda, noLoc, {}, noLoc)
335370
.get();
336371
}
@@ -341,13 +376,29 @@ namespace clad {
341376
/// more sites via DeclRefExpr + ActOnCallExpr. Use this when the same
342377
/// lambda body must be invoked from multiple paths (e.g. a reverse-pass
343378
/// segment shared between an early-return path and the natural tail).
379+
///
380+
/// The binding uses `auto` deduction so the pretty-printer renders it as
381+
/// `auto X = [&] {...};` rather than the closure type's unspellable
382+
/// `(lambda at ...)` form. Sema deduces the concrete closure type from
383+
/// the initializer; the TypeSourceInfo retains the `auto` keyword.
384+
///
385+
/// \p func emits the closure body; \p Captures then resolves that body's
386+
/// references to enclosing variables (its collect() must have already run),
387+
/// so callers hand over a pure body-emission callback.
344388
template <typename F>
345-
clang::VarDecl* buildAndBindLambda(const clang::Expr* LocE,
346-
llvm::StringRef NameHint, F&& func) {
347-
clang::Expr* lambda =
348-
buildLambda(*this, m_Sema, LocE, std::forward<F>(func));
389+
clang::VarDecl* buildAndBindLambda(const clang::Stmt* LocSrc,
390+
llvm::StringRef NameHint,
391+
LambdaCaptures& Captures, F&& func) {
392+
clang::Expr* lambda = buildLambda(*this, m_Sema, LocSrc, [&] {
393+
std::forward<F>(func)();
394+
// Resolve captures while the closure scope is active and its body is
395+
// the current block.
396+
Captures.resolve(getCurrentBlock());
397+
});
349398
clang::IdentifierInfo* II = CreateUniqueIdentifier(NameHint);
350-
return BuildVarDecl(lambda->getType(), II, lambda);
399+
clang::QualType AutoTy = m_Context.getAutoDeductType();
400+
clang::TypeSourceInfo* TSI = m_Context.getTrivialTypeSourceInfo(AutoTy);
401+
return BuildVarDecl(AutoTy, II, lambda, /*DirectInit=*/false, TSI);
351402
}
352403

353404
/// For a qualtype QT returns if it's type is Array or Pointer Type

lib/Differentiator/DerivativeBuilder.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,9 +397,11 @@ static void registerDerivative(Decl* D, Sema& S, const DiffRequest& R) {
397397

398398
clang::FunctionDecl*
399399
DerivativeBuilder::HandleNestedDiffRequest(DiffRequest& request) {
400-
// FIXME: Find a way to do this without accessing plugin namespace functions
401400
bool alreadyDerived = true;
402401
request.UpdateDiffParamsInfo(m_Sema);
402+
// Plan this lazily-scheduled request statically, so it carries the planning
403+
// info (currently the early-return flag) the static TU walk never produced.
404+
m_Scheduler.Plan(request);
403405
FunctionDecl* derivative = this->FindDerivedFunction(request);
404406
if (!derivative) {
405407
alreadyDerived = false;

lib/Differentiator/DiffPlanner.cpp

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "clang/AST/ExprObjC.h"
2525
#include "clang/AST/OperationKinds.h"
2626
#include "clang/AST/RecursiveASTVisitor.h"
27+
#include "clang/AST/Stmt.h"
2728
#include "clang/AST/Type.h"
2829
#include "clang/Analysis/AnalysisDeclContext.h"
2930
#include "clang/Basic/DiagnosticSema.h"
@@ -348,6 +349,23 @@ static QualType GetDerivedFunctionType(const CallExpr* CE) {
348349
return true;
349350
}
350351

352+
bool DiffCollector::PlanNestedRequest(DiffRequest& request) {
353+
// Lazily-scheduled requests (pushforward/pullback/higher-order, built in
354+
// DerivativeBuilder::HandleNestedDiffRequest) never pass through the static
355+
// TU walk, so their planning must be done here on demand. For now this only
356+
// records the early-return flag: m_TopMostReq is left null, so
357+
// VisitCallExpr and VisitDeclRefExpr bail and no sub-requests are spawned
358+
// -- only VisitReturnStmt runs over this request's body.
359+
// FIXME: This is where a nested request should also get its m_AnalysisDC
360+
// and the analyses currently force-disabled in HandleNestedDiffRequest.
361+
const FunctionDecl* Def =
362+
request.Function ? request.Function->getDefinition() : nullptr;
363+
if (!Def || !Def->hasBody())
364+
return true;
365+
llvm::SaveAndRestore<DiffRequest*> Saved(m_ParentReq, &request);
366+
return TraverseStmt(Def->getBody());
367+
}
368+
351369
bool DiffCollector::isInInterval(SourceLocation Loc) const {
352370
const SourceManager &SM = m_Sema.getSourceManager();
353371
for (size_t i = 0, e = m_Interval.size(); i < e; ++i) {
@@ -364,6 +382,43 @@ static QualType GetDerivedFunctionType(const CallExpr* CE) {
364382
return false;
365383
}
366384

385+
const ReturnStmt* DiffRequest::getTailReturn() const {
386+
const FunctionDecl* Def = Function ? Function->getDefinition() : nullptr;
387+
if (!Def || !Def->hasBody())
388+
return nullptr;
389+
const Stmt* Body = Def->getBody();
390+
// The tail return is the body's last statement, when that is a return.
391+
if (const auto* CS = dyn_cast<CompoundStmt>(Body))
392+
return CS->body_empty() ? nullptr
393+
: dyn_cast<ReturnStmt>(*CS->body_rbegin());
394+
return dyn_cast<ReturnStmt>(Body);
395+
}
396+
397+
bool DiffRequest::hasEarlyReturns() const {
398+
if (m_EarlyReturnInfo.HasAnalysisRun)
399+
return m_EarlyReturnInfo.HasEarlyReturns;
400+
const FunctionDecl* Def = Function ? Function->getDefinition() : nullptr;
401+
if (!Def || !Def->hasBody() || Def->getReturnType()->isVoidType())
402+
return false;
403+
// An early-return body has a return that is not the tail return. Returns
404+
// inside a nested lambda belong to that lambda's own function, so do not
405+
// descend into one.
406+
struct Finder : RecursiveASTVisitor<Finder> {
407+
const ReturnStmt* Tail = nullptr;
408+
bool Found = false;
409+
static bool TraverseLambdaExpr(LambdaExpr*) { return true; }
410+
bool VisitReturnStmt(ReturnStmt* RS) {
411+
if (RS != Tail)
412+
Found = true;
413+
return !Found; // stop at the first early return
414+
}
415+
} F;
416+
F.Tail = getTailReturn();
417+
F.TraverseStmt(Def->getBody());
418+
m_EarlyReturnInfo = {F.Found, /*HasAnalysisRun=*/true};
419+
return F.Found;
420+
}
421+
367422
void DiffRequest::UpdateDiffParamsInfo(Sema& semaRef) {
368423
// Diff info for pullbacks is generated automatically,
369424
// its parameters are not provided by the user.
@@ -1504,7 +1559,10 @@ static QualType GetDerivedFunctionType(const CallExpr* CE) {
15041559
}
15051560

15061561
bool DiffCollector::VisitDeclRefExpr(DeclRefExpr* DRE) {
1507-
if (!m_ParentReq)
1562+
// m_TopMostReq is dereferenced below; it is null when PlanNestedRequest
1563+
// walks a lazy request's body just to record its early-return flag, and no
1564+
// global-adjoint discovery is wanted there.
1565+
if (!m_ParentReq || !m_TopMostReq)
15081566
return true;
15091567
// FIXME: Add support for globals in other modes.
15101568
if (m_ParentReq->Mode != DiffMode::reverse &&

0 commit comments

Comments
 (0)