Skip to content

Commit d4fd32d

Browse files
committed
Register every cloned primal decl for reference remapping. NFC
BuildParams and DifferentiateVarDecl recorded a cloned parameter or local in m_DeclReplacements only in the awkward cases -- a nameless parameter, or a local whose clone was renamed or retyped -- and left every same-named decl to be rebound by the scope-dependent Sema::LookupName fallback in ReferencesUpdater. That name lookup is what couples cloning to the current scope and blocks cloning a reference lazily. Register the replacement for every primal parameter and local, so cloned references rebind by explicit map lookup. A suite-wide census shows the map and the name lookup never disagree, so this is behavior-preserving; it empties the fallback census in reverse mode, a step toward retiring the scope-dependent lookup entirely.
1 parent 6c842f0 commit d4fd32d

2 files changed

Lines changed: 18 additions & 26 deletions

File tree

lib/Differentiator/ReverseModeForwPassVisitor.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,9 +336,10 @@ ReverseModeForwPassVisitor::DifferentiateVarDecl(const clang::VarDecl* VD,
336336
BuildGlobalVarDecl(DerivedType, "_d_" + VD->getNameAsString(),
337337
initDiff.getExpr_dx(), VD->isDirectInit());
338338
m_Variables.emplace(VDCloned, BuildDeclRef(VDDerived));
339-
if ((VD->getDeclName() != VDCloned->getDeclName() ||
340-
DerivedType != VD->getType()))
341-
m_DeclReplacements[VD] = VDCloned;
339+
// Register the primal clone unconditionally so references rebind by map
340+
// lookup rather than the scope-dependent name-lookup fallback (see the
341+
// matching change in ReverseModeVisitor::DifferentiateVarDecl).
342+
m_DeclReplacements[VD] = VDCloned;
342343
return {VDCloned, VDDerived};
343344
}
344345

lib/Differentiator/ReverseModeVisitor.cpp

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3336,27 +3336,15 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
33363336
if (valueDx)
33373337
m_Variables.emplace(VDClone, valueDx);
33383338

3339-
// Check if decl's name is the same as before. The name may be changed
3340-
// if decl name collides with something in the derivative body.
3341-
// This can happen in rare cases, e.g. when the original function
3342-
// has both y and _d_y (here _d_y collides with the name produced by
3343-
// the derivation process), e.g.
3344-
// double f(double x) {
3345-
// double y = x;
3346-
// double _d_y = x;
3347-
// }
3348-
// ->
3349-
// double f_darg0(double x) {
3350-
// double _d_x = 1;
3351-
// double _d_y = _d_x; // produced as a derivative for y
3352-
// double y = x;
3353-
// double _d__d_y = _d_x;
3354-
// double _d_y = x; // copied from original function, collides with
3355-
// _d_y
3356-
// }
3357-
if ((VD->getDeclName() != VDClone->getDeclName() ||
3358-
VDType != VDClone->getType()))
3359-
m_DeclReplacements[VD] = VDClone;
3339+
// Register the primal clone so cloned references to VD rebind by explicit
3340+
// map lookup instead of the scope-dependent name lookup in
3341+
// ReferencesUpdater. Recording it even when the clone keeps VD's name (the
3342+
// common case) makes the map authoritative and lets the fallback retire.
3343+
// The clone's name may also differ from VD's -- it is renamed when it would
3344+
// collide with a derivation-produced name, e.g. an original `_d_y` clashing
3345+
// with the `_d_y` synthesized for `y` -- in which case the map is the only
3346+
// way to resolve the reference at all.
3347+
m_DeclReplacements[VD] = VDClone;
33603348

33613349
return DeclDiff<VarDecl>(VDClone, VDDerived);
33623350
}
@@ -5043,8 +5031,11 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
50435031
/*pushOnScopeChains=*/true,
50445032
/*cloneDefaultArg=*/false);
50455033

5046-
if (!PVD->getDeclName()) // We can't use lookup-based replacements
5047-
m_DeclReplacements[PVD] = newPVD;
5034+
// Register the primal parameter's replacement so cloned references to it
5035+
// rebind by explicit map lookup instead of the scope-dependent name
5036+
// lookup in ReferencesUpdater (a nameless param has no name to look up,
5037+
// which is why this used to be its only case).
5038+
m_DeclReplacements[PVD] = newPVD;
50485039

50495040
params.push_back(newPVD);
50505041
}

0 commit comments

Comments
 (0)