Skip to content

fix(morphizen): guard empty output NodeArg in try_fuse - #594

Open
AMDmoore wants to merge 2 commits into
mainfrom
fix/node-arg-empty-output
Open

fix(morphizen): guard empty output NodeArg in try_fuse#594
AMDmoore wants to merge 2 commits into
mainfrom
fix/node-arg-empty-output

Conversation

@AMDmoore

@AMDmoore AMDmoore commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Large hybrid/quantized models (e.g. NemotronH INT4-RTN) contain nodes with empty, non-existent NodeArg slots. Two distinct cases show up:

  • Empty first output slot (the crashing case for NemotronH): a multi-output op whose first output is unused. Concretely, NemotronH's TopK consumes only its Indices output (output 1); the Values output (output 0) is unused, so output slot 0 is an empty NodeArg.
  • Empty optional input slot: DequantizeLinear's optional zero_point in INT4-quantized weights.

Reading such a slot via node_arg_get_name fatal-CHECKs in node_arg.cpp (node_arg doesn't exist!), aborting the entire model compilation.

Fix:

  • Add node_get_first_existing_output_name() as a node utility next to node_get_first_output_name (in node.hpp / node.cpp), and use it at every try_fuse boundary that reads a node's output name (calculate_return_values, check_loop, meta_def node emission, and Pass::fuse). It skips empty/non-existent output slots and returns the first existing output name. A fused node must still produce at least one real output, so it CHECKs (matching node_get_first_output_node_arg) rather than emitting a bogus node name.
  • The isolated-node initializer collection additionally guards the empty optional input slot with node_arg_exists before reading zero_point's name.

Test plan

  • Incremental build on fix branch succeeds; hipgpu.dll / hip-compiler.dll rebuilt and deployed.
  • Llama-3.1-8B (INT4) via OGA run_oga_dynamic compiles and runs with no node_arg doesn't exist crash (MorphiZen EP Load ONNX Model Success + Compilation successful).
  • Baseline comparison: same Llama run on main produces byte-identical output, confirming zero behavioral change for models whose output slots all exist.
  • Nemotron INT4-RTN end-to-end compile (the crashing model) -- long-running (~40min), to confirm the original crash is resolved.

Made with Cursor

@github-actions

Copy link
Copy Markdown

Thanks for opening a PR!

This project follows LLVM's incremental-development and AI-tool-use
guidance. See CONTRIBUTING.md
for the project workflow.

Before requesting review, please check that:

  1. The change is focused. Substantial work links the relevant issue
    or design discussion.
  2. The PR documents relevant test results and updates affected
    documentation.
  3. If AI tools provided substantial assistance, the description
    explains what was assisted and how it was validated, and commit
    trailers identify the tool. The contributor has reviewed and
    understands the result.

Reviewers are assigned through
CODEOWNERS where ownership
is configured.

@AMDmoore
AMDmoore force-pushed the fix/node-arg-empty-output branch from 1c58746 to f58cc67 Compare July 30, 2026 11:04
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

L2 Accuracy Results (EP vs CPU)

Model Combined L2 Total Elems Skipped NaN/Inf
conv_test_hybrid 4.8668E-07 64 0
GroupQueryAttention_seq256 25.2366 2621440 0
MatMulNBits_o_seq128 259.906 368640 0
QMoE_seq128 34.957 368640 0

Threshold: 0.01 | Run: 3493 - Commit: d48fd6d

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

MorphiZen EP Performance Results

Model QPS Session (s) 1st Infer (ms) CPU% Mem (MB)
full_model_seq128 7.51 6.29 361 3 1244
GroupQueryAttention_seq128 4381.36 1.81672 11 6 311
matmul_down_seq128 526.66 2.38 72 3 352

EPContext Export Performance

Model QPS Session (s) 1st Infer (ms) CPU% Mem (MB)
full_model_seq128 7.52 46.40 362 3 15590

EPContext Import Performance

Model QPS Session (s) 1st Infer (ms) CPU% Mem (MB)
full_model_seq128 7.52 9.63 362 3 15761

OGA Benchmark Results

Model Warmup Reps Prompt Len Gen Tokens TTFT (ms) TPS Peak Mem (GB) GPU Mem (GB)
gpt-oss-20b-webgpu-int4-rtn-block-32 1 5 128 128 161.3 80.5 1.33 13.54
Llama-3.1-8B-awq-g128-int4-asym-fp16-onnx-dml 1 5 128 128 335.8 44.1 1.22 6.43

OGA Wheel Smoke (Python benchmark_e2e.py)

Model TTFT (ms) TPS
Llama-3.1-8B-awq-g128-int4-asym-fp16-onnx-dml 196 43.1

Run: 3493 - Commit: d48fd6d

Large hybrid/quantized models (e.g. NemotronH INT4-RTN) contain nodes whose
first output slot is an empty, non-existent NodeArg -- optional outputs and
DequantizeLinear's optional zero_point in INT4-quantized weights. Reading such
a slot via node_arg_get_name fatal-CHECKs in node_arg.cpp
("node_arg doesn't exist!"), aborting the whole compilation.

Add node_first_existing_output_name() and guard the try_fuse boundary
(calculate_return_values, check_loop, isolated-node initializer collection,
meta_def node emission, and Pass::fuse) so empty/optional output slots fall
back to the first existing output name instead of crashing.

Co-Authored-By: Cursor <cursor@cursor.com>
Made-with: Cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread morphizen/morphizen-core/src/pass_imp.cpp Outdated
Comment thread morphizen/morphizen-core/src/pass.cpp Outdated
@AMDmoore
AMDmoore force-pushed the fix/node-arg-empty-output branch from f58cc67 to 259cef8 Compare July 30, 2026 16:31
AMDmoore added a commit that referenced this pull request Jul 31, 2026
Address review feedback on PR #594:
- Move node_first_existing_output_name next to node_get_first_output_name
  in the node utilities (node.hpp / node.cpp) so both pass.cpp and
  pass_imp.cpp reuse it instead of duplicating the first-existing-output
  loop.
- Pass a NodeConstRef (node_ref) at both call sites.

Co-authored-by: Cursor <cursoragent@cursor.com>
AMDmoore added a commit that referenced this pull request Jul 31, 2026
Address review feedback on PR #594:
- Move node_first_existing_output_name next to node_get_first_output_name
  in the node utilities (node.hpp / node.cpp) so both pass.cpp and
  pass_imp.cpp reuse it instead of duplicating the first-existing-output
  loop.
- Pass a NodeConstRef (node_ref) at both call sites.

Co-authored-by: Cursor <cursoragent@cursor.com>
@AMDmoore
AMDmoore force-pushed the fix/node-arg-empty-output branch 2 times, most recently from 122e0bb to d48fd6d Compare July 31, 2026 10:45
Address review feedback on PR #594:
- Move node_get_first_existing_output_name next to node_get_first_output_name
  in the node utilities (node.hpp / node.cpp) so both pass.cpp and
  pass_imp.cpp reuse it instead of duplicating the first-existing-output
  loop.
- At the NodeConstRef call sites, pass the underlying const Node&
  explicitly via *node_ref.ptr() rather than relying on the implicit
  NodeConstRef -> const Node& conversion.

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

2 participants