Skip to content

[compiler] isolate mutable state across dynamic if branches - #1003

Merged
coderfeli merged 1 commit into
ROCm:mainfrom
Deep-Spark:kefan.cao/list-branch-state
Aug 14, 2026
Merged

[compiler] isolate mutable state across dynamic if branches#1003
coderfeli merged 1 commit into
ROCm:mainfrom
Deep-Spark:kefan.cao/list-branch-state

Conversation

@kefan203

@kefan203 kefan203 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

[compiler] isolate mutable state across dynamic if branches

Motivation

Dynamic if already carries Python containers (list / dict /
SimpleNamespace / nested) as scf.if results by unpacking their leaves into
SSA values. That is enough for whole-container rebinding such as
lst = [a, b], but not for in-place updates:

lst = [fx.Int32(1), fx.Int32(2)]
if flag > fx.Int32(0):
    lst[0] = fx.Int32(10)

Both branches of scf_if_dispatch previously received the same Python
container object. During tracing, an in-place write in the then-branch mutated
the shared object before the else-branch ran, so the else-branch observed the
updated element even though its SSA path never assigned it. The resulting
scf.if therefore merged contaminated state and produced wrong values when the
condition was false.

This PR clones the mutable container structure once per branch before the
branch body runs, so in-place updates stay local to that branch’s tracing
inputs. DSL / IR leaves are shared by identity (they are already SSA-managed);
only the Python container skeleton is duplicated.

Technical Details

  • Add _clone_mutable_state in python/flydsl/compiler/ast_rewriter.py.
    It deep-clones list, dict, SimpleNamespace, and tuple structure while
    preserving cycles via an id-keyed memo and leaving non-container leaves
    (DSL values, IR values, scalars) untouched.
  • In ReplaceIfWithDispatch.scf_if_dispatch, each dynamic branch now receives
    branch_inputs = _clone_mutable_state(result_values) instead of the shared
    entry result_values.
  • _normalize_branch_result falls back through that per-branch map, so a
    name omitted from a branch return dict resolves to the branch’s own clone,
    not the contaminated entry container.

Test Plan

  • Unit: tests/unit/test_dynamic_controlflow_list_carry.py::test_if_inplace_list_updates_do_not_contaminate_branches
    asserts then/else receive distinct clones, the entry list is untouched, and
    the else-branch still sees the original element after the then-branch stores.
  • E2E: tests/system/test_dynamic_controlflow_list_carry_e2e.py
    • test_if_inplace_list_taken[10, 2]
    • test_if_inplace_list_not_taken[1, 2]
  • Existing list-carry unit / e2e suites for rebinding and nested containers.

Test Result

  • New unit + e2e inplace-list tests pass.
  • Existing dynamic control-flow list-carry suites remain green.

Submission Checklist

@kefan203

Copy link
Copy Markdown
Contributor Author

Hi @coderfeli @sjfeng1999 , please help review this PR

@coderfeli
coderfeli requested a review from xudoyuan August 13, 2026 02:50
Comment thread python/flydsl/compiler/ast_rewriter.py Outdated
Comment thread python/flydsl/compiler/ast_rewriter.py Outdated
Rebuild Python container state from carried IR values for each branch so in-place updates during tracing cannot leak into sibling branches.
@kefan203
kefan203 force-pushed the kefan.cao/list-branch-state branch from 4612cf4 to 13fb55f Compare August 14, 2026 04:26
@kefan203
kefan203 requested a review from xudoyuan August 14, 2026 05:13

@xudoyuan xudoyuan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kefan203

Copy link
Copy Markdown
Contributor Author

Hi @xudoyuan @coderfeli , all CI checks have passed now. Could you please help merge this PR when you have time? Thanks!

@xudoyuan
xudoyuan requested a review from sjfeng1999 August 14, 2026 09:34
@coderfeli
coderfeli merged commit 01d63f7 into ROCm:main Aug 14, 2026
12 checks passed
@kefan203
kefan203 deleted the kefan.cao/list-branch-state branch August 17, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants