Skip to content

Conversation

@meenakshiramanathan1
Copy link
Contributor

@meenakshiramanathan1 meenakshiramanathan1 commented Aug 18, 2025

Ticket

Problem description

  • Topk op cannot be directly supported in forge since it's returns values and inidices as outputs while forge only supports ops with single outputs.

What's changed

This PR adds support for the TopK operation (returning both values and indices) across the stack and also updates the graph builder and tensor utilities to better handle ops with multiple outputs. Core changes are mentioned below.

  • compile.py: Ensures a single IR op node per source op name and reuses it across multiple consumers/outputs and while creating tensors to build the graph, caches created op nodes by tensor.src_op.name and reuses them instead of creating duplicates and keeps output naming unique and stable so both TopK outputs can attach to the same underlying op node.

  • tvm_to_python.py: It Maps topk to forge and when consuming a TopK node’s indices (producer port 1), rewrites the input name to <topk_node_name>_indices and when a TopK node is a graph output, adds a secondary output name _indices so both values and indices are returned.

  • python_codegen.py: Modifies forgewriter to emit proper LHS unpacking for TopK and generates values, values_indices.

  • topk.py: Adds a Python-friendly layer and tuple-like wrapper for TopK. TensorPair wraps (values, indices) and behaves like a 2‑tuple, forwarding attributes to values and supporting set_src_layer for both. It computes golden outputs with torch.topk when input has values, otherwise creates correctly-shaped zero tensors and it creates Forge tensors from trace and sets their values and data formats to returns a TensorPair.

  • tensor.py: Extends conversion utilities to accept tuple-like wrappers (e.g., TensorPair) when converting to Forge tensors and detects iterable/sequence-like wrappers and flattens them into the corresponding list of Tensors.

The generated graph now unpacks two values from topk.
Screenshot 2025-08-19 at 10 50 54 AM
Screenshot 2025-08-19 at 1 55 08 PM

  • Topk op sanity passes tvm verification successfully and further fails in run_mlir_compiler_stage with unsupported op topk.

Logs

topk_pcc_correction.log

@meenakshiramanathan1 meenakshiramanathan1 force-pushed the mramanathan/topk_multi_op branch 6 times, most recently from 652b6e8 to 7d8b3b7 Compare August 18, 2025 13:51
@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2025

Codecov Report

❌ Patch coverage is 4.87805% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.11%. Comparing base (f57530e) to head (4350656).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
forge/csrc/ops/op_topk.cpp 0.00% 27 Missing ⚠️
forge/csrc/ops/op.cpp 14.28% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2830      +/-   ##
==========================================
- Coverage   62.29%   62.11%   -0.19%     
==========================================
  Files         157      158       +1     
  Lines       12839    12880      +41     
==========================================
+ Hits         7998     8000       +2     
- Misses       4841     4880      +39     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@meenakshiramanathan1 meenakshiramanathan1 force-pushed the mramanathan/topk_multi_op branch 9 times, most recently from 813c166 to 4a5549e Compare August 19, 2025 07:37
@meenakshiramanathan1 meenakshiramanathan1 force-pushed the mramanathan/topk_multi_op branch 2 times, most recently from 0ec7229 to 88bc0f5 Compare August 19, 2025 07:49
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.

Add support for topk op

3 participants