Skip to content

fix(lowering): order if/else phi and yield names by first definition - #777

Merged
zhenrongliew merged 1 commit into
mainfrom
dl/lowering-deterministic-phi-order
Oct 13, 2026
Merged

zhenrongliew merged 1 commit into
mainfrom
dl/lowering-deterministic-phi-order

Conversation

@zhenrongliew

Copy link
Copy Markdown
Collaborator

Fixes #775.

lower_If in both the cf and scf lowerings collected the names that need a join argument (cf) or a yield (scf) in a set[str], then built the IR by iterating it. String hashes are randomized per process, so the same kernel lowered to IR with a different argument/yield order in each process. Each lowering was still correct, because every loop iterates the same set, but the IR text and any hash of it changed between runs.

This causes misses in the on-disk compile cache (#769). The key is computed from the lowered IR, so a new process often computes a new key for an unchanged kernel, then recompiles it and saves another copy. Callers are affected too, because a kernel's key includes its callees' keys.

Changes

  • dialects/lowering/cf.py: phi is a dict[str, None] instead of a set[str].
  • dialects/scf/lowering.py: all_names = dict.fromkeys([*body_frame.defs, *else_frame.defs]).

Names now come out in the order they are first defined: the condition, then the then-branch's names, then names assigned only in the else branch.

Tests

  • New test_ifelse_order_independent_of_hash_seed (test/lowering/test_ifelse.py) lowers one if/else with basic_no_opt and structural_no_opt in fresh processes under 4 hash seeds, and checks that every run gives ['flag', 'x', 'y']. On main it fails: 4 seeds give 4 different orders.
  • The full suite passes under several hash seeds.

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-13 13:18 UTC

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

☂️ Code Coverage

current status: ✅

Overall Coverage

Statements Covered Coverage Threshold Status
12307 11132 90% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
src/kirin/dialects/lowering/cf.py 96% 🟢
src/kirin/dialects/scf/lowering.py 96% 🟢
TOTAL 96% 🟢

updated for commit: ca252de by action🐍

@codecov

codecov Bot commented Oct 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@zhenrongliew
zhenrongliew merged commit 9be49b5 into main Oct 13, 2026
11 checks passed
@zhenrongliew
zhenrongliew deleted the dl/lowering-deterministic-phi-order branch October 13, 2026 13:17
@github-actions

Copy link
Copy Markdown
Contributor

Backport results for 9be49b5

Succeeded:

zhenrongliew pushed a commit that referenced this pull request Oct 13, 2026
Automated backport of PR #777 (9be49b5)
to `release-0-22`.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

if/else lowering orders phi arguments by string hash, so the IR changes with PYTHONHASHSEED

2 participants