Bind ifelse_branching to a CSE temporary while keeping branches lazy#966
Draft
ChrisRackauckas-Claude wants to merge 2 commits into
Draft
Bind ifelse_branching to a CSE temporary while keeping branches lazy#966ChrisRackauckas-Claude wants to merge 2 commits into
ifelse_branching to a CSE temporary while keeping branches lazy#966ChrisRackauckas-Claude wants to merge 2 commits into
Conversation
`cse_inside_expr = false` previously conflated "do not descend into the arguments" with "do not bind this node", so an `ifelse_branching` referenced at multiple sites re-emitted its entire `if`/`else` per use. Add a second hook, `cse_bind_expr`, consulted only when `cse_inside_expr` is `false`: when it returns `true`, CSE binds the node as-is (arguments un-CSEd) to a temporary, so all references share one computation. `ifelse_branching` opts in; the default `false` preserves the existing fully-inline behaviour for `Operator`, `getindex` and any other opt-outs. The branches remain lazy (a conditional inside a branch is still emitted within that branch), and one-sided nesting now stays linear, matching `ifelse`. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
This was referenced Jun 12, 2026
Contributor
Benchmark Results (Julia vlts)Time benchmarks
Memory benchmarks
|
Contributor
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
Contributor
Author
|
CI triage — 26/29 green (all SymbolicUtils own-test matrices,
|
Contributor
Author
|
The |
AayushSabharwal
approved these changes
Jun 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #965 (this was developed on that PR's branch but GitHub dropped the push's synchronize event, so it never attached before the merge — re-landed here on current master).
Multiply-referenced
ifelse_branchingnow shares a singleif/elseinstead of re-emitting it per use site. Previously,cse_inside_expr = falseconflated "do not descend into the arguments" with "do not bind this node": keeping the branches lazy also un-bound the conditional, so e.g.sin(z) + cos(z)withz = ifelse_branching(...)evaluated the taken branch twice.Implementation
A second hook,
Code.cse_bind_expr(sym, f)(defaultfalse), consulted only whencse_inside_exprisfalse. Whentrue,_cse_computebinds the node as-is — arguments un-CSEd — to a CSE temporary;cse!'s id-cache makes every reference in the scope resolve to that one temporary.ifelse_branchingopts in.Operator,getindexand any other opt-outs keep their exact prior fully-inline behaviour (theircse(ex) == exidentity tests are untouched and pass).Generated code for the repro is now:
One semantic boundary (asserted with a comment in the tests): a conditional referenced inside both branches of an enclosing
ifelse_branchingis still emitted per branch — hoisting it above the conditional would evaluate it eagerly, which the laziness contract forbids. One-sided nesting is linear, matchingifelse(previously exponential in depth).Verification
test/conditionals.jl: side-effect counting shows the taken branch fires once (was twice) and the untaken branch never; the CSE structure contains exactly one boundifelse_branching; one-sided nesting emits the same number ofifs asifelse; values matchifelsethroughout.Pkg.test()on this branch (on top of the add some more specialized hashing functions #963 hashing changes): 17771 pass, 3 broken (pre-existing), 0 fail.cse_bind_exprdocumented and added to the codegen manual alongsidecse_inside_expr, plus docs entries for the two operators.Version bumped to 4.36.0. Companion test on the Symbolics side: JuliaSymbolics/Symbolics.jl#1892 (blocked on this releasing).
🤖 Generated with Claude Code