fix(codegen): emit tensor phis via tensors dict to avoid NameError (#2180) by georgebisbas · Pull Request #2183 · hw-native-sys/pypto

georgebisbas · 2026-07-28T10:57:46Z

Summary

Fixes #2180: cross-branch phi NameError in host_orch.py at prepare() time.

When ConvertToSSA synthesizes phi return_vars_ for cross-branch diverging tensor variables, the distributed codegen now:

Pre-declares phi variables in the tensors dict before the if block
Emits yield-to-phi assignments via tensors[...] = tensors[...] (not bare Python names)
Traces Var→Var tensor aliases (kernel param copies) to the original tensors-dict reference
Emits tensor-to-tensor AssignStmts as tensors["var"] = tensors["value"] instead of bare Python variable names

Test plan

New unit test test_if_cross_branch_phi_predeclares_and_yields_tensors validates:
- Phi pre-declaration before the if block
- Both then-branch and else-branch yield-to-phi assignments use tensors[...]
- No bare-Python-name tensor assignments remain in branches
All 27 existing distributed codegen unit tests pass
All 125 SSA transformation tests pass (no regressions)
Generated host_orch.py confirmed to produce valid, executable code that no longer raises NameError

Co-authored-by: @georgebisbas @vloncar

When ConvertToSSA synthesizes phi `return_vars_` for cross-branch diverging tensor variables, host_orch.py codegen must pre-declare the phi name in the `tensors` dict and emit yield-to-phi assignments that reference tensors-dict names — not bare Python variables. Fixes hw-native-sys#2180. - IfStmt visitor (DistributedCodegen): pre-declares phi variables before `if`, emits branch-specific yield-to-phi `tensors[…] = tensors[…]` assignments, and traces Var→Var aliases (kernel param copies) to the original tensors-dict name. - AssignStmt visitor: aliases `tensor_var = kernel_param` are emitted as `tensors["tensor_var"] = tensors["kernel_param"]` instead of bare Python names. - Unit test validates phi pre-declaration, yield assignments, and absence of bare-name tensor assignments. Co-authored-by: georgebisbas <georgios.bismpas@h-partners.com> Co-authored-by: vloncar <vloncar@users.noreply.github.com>

coderabbitai · 2026-07-28T10:58:07Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: acb8c51e-049f-4df3-8a00-e03548506580

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

Distributed codegen now preserves tensor aliases through the tensors registry and emits SSA phi declarations and branch assignments for conditional control flow. Tests optionally run SSA conversion and verify generated host orchestration code for cross-branch tensor values.

Changes

Distributed tensor phi handling

Layer / File(s)	Summary
Tensor alias assignment handling `src/codegen/distributed/distributed_codegen.cpp`	Tensor-to-tensor variable assignments now emit `tensors[lhs] = tensors[rhs]` and update codegen tracking state.
Conditional SSA phi emission `src/codegen/distributed/distributed_codegen.cpp`, `tests/ut/codegen/distributed/test_host_orch_distributed.py`	Conditional branches now pre-declare phi variables, resolve yielded sources, and assign tensor or scalar phi values per branch; tests add optional SSA conversion and validate generated tensor assignments.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

#2179 — Covers the same distributed codegen cross-branch tensor-phi NameError behavior addressed here.

Poem

I hopped through branches, soft and bright,
And tucked each tensor safely right.
Phi names bloom before if skies,
Each yielded value finds its rise.
No missing names—just carrots galore! 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is specific and matches the main fix: emitting tensor phis through the tensors dict to avoid NameError.
Description check	✅ Passed	The description is clearly about the same cross-branch tensor phi NameError fix and added validation.
Linked Issues check	✅ Passed	The changes address `#2180` by predeclaring phi tensors, assigning branch yields through tensors, and adding a regression test.
Out of Scope Changes check	✅ Passed	The extra alias handling and tensor-to-tensor assignment logic support the same phi and tensors-dict fix, so they are in scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ecbf7e1acd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-28T11:03:40Z

+            VisitExpr(src);
+            tensor_phi_init = current_expr_value_;


Avoid emitting branch calls while probing phi init

When a HOST orchestrator has no tensor formal parameters, this fallback tries to derive a tensor phi initializer by visiting the then-branch assignment RHS before emitting the if. If that RHS is a hierarchy call such as boundary = self.chip_run(tmp) (where tmp is a top-level created tensor), VisitExpr(src) calls EmitCallToWorker, so the generated host_orch.py submits that chip task unconditionally before the branch condition is evaluated, and with no assignment target for the call output. That changes program behavior for no-input host orchestrators with tensor phis; initializer discovery here needs to avoid non-pure expression visitors and only use already-materialized tensor names.

Useful? React with 👍 / 👎.

Fixed in 8e0e5e0: both init-discovery paths now guard VisitExpr with ir::As<ir::Var>(src) != nullptr so only Var→Var aliases are visited. For Call RHS the init falls through to the shared tensor_phi_init (function params) or zero placeholder — EmitCallToWorker is never triggered before the if condition.

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/codegen/distributed/distributed_codegen.cpp`:
- Around line 869-909: Update the tensor-phi handling in the pre-declaration
loop over op->return_vars_ so each tensor phi derives its initializer from that
phi’s own in-scope source or pre-if tensor, rather than reusing the single
tensor_phi_init selected earlier. Preserve the initializer’s shape and dtype,
and only use the fallback placeholder when no valid source exists for that
specific phi.

In `@tests/ut/codegen/distributed/test_host_orch_distributed.py`:
- Around line 959-1020: Strengthen the then_yield assertion in
test_if_cross_branch_phi_predeclares_and_yields_tensors so it examines only the
generated then-branch body, excluding the pre-if phi declaration. Isolate the
region between the if and else boundaries (or otherwise anchor the match after
the if) and require the tensors-based phi assignment there, so the assertion
fails if then-branch emission regresses.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6a4b9206-d633-4f0b-a013-142cf86d5458

📥 Commits

Reviewing files that changed from the base of the PR and between 478ddad and ecbf7e1.

📒 Files selected for processing (2)

src/codegen/distributed/distributed_codegen.cpp
tests/ut/codegen/distributed/test_host_orch_distributed.py

…-sys#2183) CodeRabbit review: each tensor-typed phi now derives its pre-declaration initializer from its own yield source instead of a shared init. Test assertion isolates the then-branch body from the pre-if declaration. Pre-commit: suppress pyright reportReturnType for IR-level return 0.

…w-native-sys#2183) When a HOST orchestrator has no tensor formal parameters, the phi init fallback visited the then-branch yield source via VisitExpr. If that source is a hierarchy call, EmitCallToWorker would emit a _submit_chip before the `if` condition, altering program semantics. Only resolve Var→Var aliases now; Call RHS falls through to the zero placeholder. Co-authored-by: georgebisbas <georgios.bismpas@h-partners.com> Co-authored-by: vloncar <vloncar@users.noreply.github.com>

github-project-automation Bot added this to pto project Jul 28, 2026

chatgpt-codex-connector Bot reviewed Jul 28, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 28, 2026

View reviewed changes

Comment thread src/codegen/distributed/distributed_codegen.cpp

Comment thread tests/ut/codegen/distributed/test_host_orch_distributed.py

georgebisbas and others added 2 commits July 28, 2026 13:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(codegen): emit tensor phis via tensors dict to avoid NameError (#2180) - #2183

fix(codegen): emit tensor phis via tensors dict to avoid NameError (#2180)#2183
georgebisbas wants to merge 3 commits into
hw-native-sys:mainfrom
georgebisbas:fix/issue-2180-phi-codegen-nameerror

georgebisbas commented Jul 28, 2026

Uh oh!

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 28, 2026

Uh oh!

georgebisbas Jul 28, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

georgebisbas commented Jul 28, 2026

Summary

Test plan

Uh oh!

coderabbitai Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

georgebisbas Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading