Skip to content

Commit 53ebd00

Browse files
committed
Reverse switches on the stored condition instead of a control-flow tape
Reverse-mode differentiation of a switch statement recorded, in a second control-flow tape, which case a `break` exited from, then re-dispatched the reverse sweep by popping that tape. The information is redundant: the switch condition is already stored (`_cond`), and every case guard already compares against it (`if (v == _cond) break`). The extra tape -- and the BreakContStmtHandler state backing it -- only duplicated what the condition carries. Drop the control-flow tape for switches and re-switch on the stored condition directly. Each fall-through group's reverse entry is now labelled with its original case values rather than a synthesized counter, and the trailing group (closed by the switch end rather than a break) is labelled in VisitSwitchStmt. The per-case guards are unchanged. Loops keep their control-flow tape, where a break's iteration genuinely cannot be recovered from a condition. This is behaviour-preserving: all Switch.C/SwitchInit.C execution results are unchanged; only the generated code -- forward (the counter pushes are gone) and reverse -- and its FileCheck baselines change, with one fewer tape. A switch whose cases return rather than break is added to Switch.C to cover the returning-case shape. SwitchInit.C no longer needs its Valgrind XFAIL: the control-flow tape it tripped on under memcheck is gone.
1 parent cf64927 commit 53ebd00

4 files changed

Lines changed: 343 additions & 217 deletions

File tree

include/clad/Differentiator/ReverseModeVisitor.h

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -703,17 +703,18 @@ namespace clad {
703703

704704
StmtDiff DifferentiateCanonicalLoop(const clang::ForStmt* S);
705705

706-
/// This class modifies forward and reverse blocks of the loop/switch
707-
/// body so that `break` and `continue` statements are correctly
708-
/// handled. `break` and `continue` statements are handled by
709-
/// enclosing entire reverse block loop body in a switch statement
710-
/// and only executing the statements, with the help of case labels,
711-
/// that were executed in the associated forward iteration. This is
712-
/// determined by keeping track of which `break`/`continue` statement
713-
/// was hit in which iteration and that in turn helps to determine which
714-
/// case label should be selected.
715-
///
716-
/// Class usage:
706+
/// Handles `break`/`continue` inside a differentiated loop. It owns a
707+
/// control-flow tape recording which one fired in which iteration, so the
708+
/// reverse loop body -- wrapped in a switch over that tape -- replays
709+
/// exactly the statements the forward iteration executed. The members below
710+
/// serve this tape.
711+
///
712+
/// One handler is pushed per enclosing loop or switch, so the top of the
713+
/// stack is the innermost construct a `break` binds to. A switch sets
714+
/// m_IsInvokedBySwitchStmt and leaves the tape unused -- its reverse is
715+
/// rebuilt from the stored condition instead (VisitSwitchStmt).
716+
///
717+
/// Loop usage:
717718
///
718719
/// ```cpp
719720
/// auto activeBreakContStmtHandler = PushBreakContStmtHandler();
@@ -760,6 +761,9 @@ namespace clad {
760761
clang::Expr* CreateCFTapePushExpr(std::size_t value);
761762

762763
public:
764+
/// True when the innermost breakable construct is a source-level switch,
765+
/// whose `break`s are handled in VisitSwitchStmt rather than by the
766+
/// control-flow tape. Lets VisitBreakStmt pick the right handling.
763767
bool m_IsInvokedBySwitchStmt = false;
764768

765769
BreakContStmtHandler(ReverseModeVisitor& RMV, bool forSwitchStmt = false)
@@ -835,9 +839,18 @@ namespace clad {
835839

836840
/// Stores data required for differentiating a switch statement.
837841
struct SwitchStmtInfo {
842+
/// The forward-pass case/default labels, in source order.
838843
llvm::SmallVector<clang::SwitchCase*, 16> cases;
844+
/// The stored switch condition (`_cond`), reused both as the reverse
845+
/// switch discriminator and by every case guard.
839846
clang::Expr* switchStmtCond = nullptr;
840847
clang::IfStmt* defaultIfBreakExpr = nullptr;
848+
/// Index into `cases` of the first label of the fall-through group not
849+
/// yet closed by a `break`.
850+
std::size_t groupStart = 0;
851+
/// The reverse switch's entry labels, built from the original case values
852+
/// rather than a control-flow tape counter.
853+
llvm::SmallVector<clang::SwitchCase*, 16> reverseEntryCases;
841854
};
842855

843856
/// Maintains a stack of `SwitchStmtInfo`.
@@ -854,6 +867,12 @@ namespace clad {
854867

855868
void PopSwitchStmtInfo() { m_SwitchStmtsData.pop_back(); }
856869

870+
/// Closes the currently open fall-through group `cases[groupStart..)`:
871+
/// builds its reverse-switch entry from the group's original case labels,
872+
/// registers them in `reverseEntryCases`, advances `groupStart`, and
873+
/// returns the label chain to prepend before the group's adjoint replay.
874+
clang::Stmt* CloseReverseSwitchCaseGroup(SwitchStmtInfo& SSData);
875+
857876
private:
858877
// When differentiating ArrayInitLoopExpr, we need to replace
859878
// ArrayInitIndexExpr with real indices. We need to both add and pop them in

lib/Differentiator/ReverseModeVisitor.cpp

Lines changed: 71 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4412,6 +4412,8 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
44124412
// statement body will be processed in both the forward and the reverse
44134413
// pass. Thus, we do not need to add them in the differentiated function.
44144414
if (!(SSData->cases.empty())) {
4415+
// The forward sweep is a clone of the original switch: control flow is
4416+
// recorded implicitly by the stored condition, so no tape is needed.
44154417
Sema::ConditionResult condRes =
44164418
m_Sema.ActOnCondition(getCurrentScope(), noLoc, CloneNode(condExpr),
44174419
Sema::ConditionKind::Switch);
@@ -4421,18 +4423,40 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
44214423
/*LParenLoc=*/noLoc, nullptr, condRes,
44224424
/*RParenLoc=*/noLoc)
44234425
.getAs<SwitchStmt>();
4424-
activeBreakContHandler->UpdateForwAndRevBlocks(bodyDiff);
4425-
4426-
// Registers all the cases to the switch statement.
44274426
for (auto* SC : SSData->cases)
44284427
forwardSS->addSwitchCase(SC);
4429-
44304428
forwardSS =
44314429
m_Sema.ActOnFinishSwitchStmt(noLoc, forwardSS, bodyDiff.getStmt())
44324430
.getAs<SwitchStmt>();
44334431

4432+
// The reverse sweep re-switches on the stored condition. Each
4433+
// fall-through group's adjoint replay is entered through the original
4434+
// case values (the per-case `if (v == _cond) break` guards emitted by
4435+
// VisitCaseStmt peel off the cases that did not run). The trailing group
4436+
// is closed by the switch end rather than a break, so label it here; it
4437+
// is the topmost group in the bottom-up reverse block.
4438+
Stmt* revBody = bodyDiff.getStmt_dx();
4439+
if (SSData->groupStart < SSData->cases.size()) {
4440+
Stmt* finalEntry = CloseReverseSwitchCaseGroup(*SSData);
4441+
revBody =
4442+
utils::PrependAndCreateCompoundStmt(m_Context, revBody, finalEntry);
4443+
}
4444+
Sema::ConditionResult revCondRes =
4445+
m_Sema.ActOnCondition(getCurrentScope(), noLoc, CloneNode(condExpr),
4446+
Sema::ConditionKind::Switch);
4447+
SwitchStmt* reverseSS =
4448+
m_Sema
4449+
.ActOnStartOfSwitchStmt(/*SwitchLoc=*/noLoc,
4450+
/*LParenLoc=*/noLoc, nullptr, revCondRes,
4451+
/*RParenLoc=*/noLoc)
4452+
.getAs<SwitchStmt>();
4453+
for (auto* SC : SSData->reverseEntryCases)
4454+
reverseSS->addSwitchCase(SC);
4455+
reverseSS = m_Sema.ActOnFinishSwitchStmt(noLoc, reverseSS, revBody)
4456+
.getAs<SwitchStmt>();
4457+
44344458
addToCurrentBlock(forwardSS, direction::forward);
4435-
addToCurrentBlock(bodyDiff.getStmt_dx(), direction::reverse);
4459+
addToCurrentBlock(reverseSS, direction::reverse);
44364460
}
44374461

44384462
PopBreakContStmtHandler();
@@ -4492,6 +4516,36 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
44924516
return {endBlock(direction::forward), endBlock(direction::reverse)};
44934517
}
44944518

4519+
Stmt*
4520+
ReverseModeVisitor::CloseReverseSwitchCaseGroup(SwitchStmtInfo& SSData) {
4521+
// Build `case v1: case v2: ... [default:] ;` for the still-open group's
4522+
// original labels, innermost first. The order of the labels is irrelevant
4523+
// (they all fall into the same reverse replay); the shared null
4524+
// substatement lets the group's adjoints follow as siblings in the switch
4525+
// body.
4526+
Stmt* inner = m_Sema.ActOnNullStmt(noLoc).get();
4527+
for (std::size_t i = SSData.groupStart, e = SSData.cases.size(); i != e;
4528+
++i) {
4529+
SwitchCase* rev = nullptr;
4530+
if (isa<DefaultStmt>(SSData.cases[i])) {
4531+
rev = new (m_Context) DefaultStmt(noLoc, noLoc, inner);
4532+
} else {
4533+
auto* fwd = cast<CaseStmt>(SSData.cases[i]);
4534+
// Clone the range high end too, matching the forward case, so a GNU
4535+
// `case a ... b:` keeps its extent in the reverse switch.
4536+
Expr* rhs = fwd->getRHS() ? CloneNode(fwd->getRHS()) : nullptr;
4537+
auto* caseStmt = CaseStmt::Create(m_Context, CloneNode(fwd->getLHS()),
4538+
rhs, noLoc, noLoc, noLoc);
4539+
caseStmt->setSubStmt(inner);
4540+
rev = caseStmt;
4541+
}
4542+
SSData.reverseEntryCases.push_back(rev);
4543+
inner = rev;
4544+
}
4545+
SSData.groupStart = SSData.cases.size();
4546+
return inner;
4547+
}
4548+
44954549
static bool hasCheckpointingPragma(ASTContext& C, SourceLocation loopLoc,
44964550
const DiffRequest& request) {
44974551
if (!loopLoc.isValid())
@@ -4640,10 +4694,18 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
46404694
Stmt* newBS =
46414695
clad_compat::ActOnBreakStmt(m_Sema, noLoc, getCurrentScope()).get();
46424696
auto* activeBreakContHandler = GetActiveBreakContStmtHandler();
4697+
// A break in a source-level switch only closes the current fall-through
4698+
// group (VisitSwitchStmt turns each group into a reverse-switch entry).
4699+
// Unlike the loop path below, it records nothing on a control-flow tape.
4700+
if (activeBreakContHandler->m_IsInvokedBySwitchStmt) {
4701+
addToCurrentBlock(newBS);
4702+
Stmt* revEntry = CloseReverseSwitchCaseGroup(*GetActiveSwitchStmtInfo());
4703+
return {endBlock(direction::forward), revEntry};
4704+
}
46434705
Stmt* CFCaseStmt = activeBreakContHandler->GetNextCFCaseStmt();
46444706
Stmt* pushExprToCurrentCase = activeBreakContHandler
46454707
->CreateCFTapePushExprToCurrentCase();
4646-
if (isInsideLoop && !activeBreakContHandler->m_IsInvokedBySwitchStmt) {
4708+
if (isInsideLoop) {
46474709
Expr* tapeBackExprForCurrentCase =
46484710
activeBreakContHandler->CreateCFTapeBackExprForCurrentCase();
46494711
if (m_CurrentBreakFlagExpr) {
@@ -4733,7 +4795,9 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
47334795

47344796
void ReverseModeVisitor::BreakContStmtHandler::UpdateForwAndRevBlocks(
47354797
StmtDiff& bodyDiff) {
4736-
if (m_SwitchCases.empty() && !m_IsInvokedBySwitchStmt)
4798+
// Only loops reach here; a loop with no break/continue needs no
4799+
// control-flow switch in its reverse body.
4800+
if (m_SwitchCases.empty())
47374801
return;
47384802

47394803
// Add case statement in the beginning of the reverse block

0 commit comments

Comments
 (0)