Skip to content

docs(rocke): document C++ touchpoints for adding an IR op - #10801

Merged
CongMa13 merged 1 commit into
developfrom
users/congma/ck/aick-1711-add-ir-op-docs
Sep 14, 2026
Merged

CongMa13 merged 1 commit into
developfrom
users/congma/ck/aick-1711-add-ir-op-docs

Conversation

@CongMa13

Copy link
Copy Markdown
Contributor

Expand dsl_docs/development/extending.md section 1 with the full C++ side of adding an IR op: the rocke_b_ builder, the rocke_opcode_t enum, and the three parallel per-opcode tables (rocke_opcode_names[], rocke_opcode_pure[], and the verify.cpp arity classifier) that must stay in enumerator order. Correct the purity description to reflect the PURE_OP_NAMES allowlist model, and fix stale example paths (example/ck_tile/dsl -> python/rocke/examples).

JIRA ID : AICK-1711

@therock-pr-bot

therock-pr-bot Bot commented Aug 14, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

Expand dsl_docs/development/extending.md section 1 with the full C++ side of
adding an IR op: the rocke_b_<op> builder, the rocke_opcode_t enum, and the
three parallel per-opcode tables (rocke_opcode_names[], rocke_opcode_pure[],
and the verify.cpp arity classifier) that must stay in enumerator order.
Correct the purity description to reflect the PURE_OP_NAMES allowlist model,
and fix stale example paths (example/ck_tile/dsl -> python/rocke/examples).

JIRA ID : AICK-1711

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CongMa13
CongMa13 force-pushed the users/congma/ck/aick-1711-add-ir-op-docs branch from 9284f6e to cd0ed75 Compare August 14, 2026 16:41
@CongMa13
CongMa13 marked this pull request as ready for review August 14, 2026 16:41
@CongMa13
CongMa13 requested review from a team as code owners August 14, 2026 16:41
@cgmillette
cgmillette requested review from yraparti and a lite review from Copilot August 20, 2026 08:43

@cgmillette cgmillette left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm - would like second opinion from @yraparti

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates Rocke DSL extension documentation to better describe the end-to-end steps for adding a new IR op, including the C++ engine touchpoints and parity expectations between Python/C++ lowering paths.

Changes:

  • Expands “Adding a New IR Operation” with a 7-touchpoint checklist, including C++ IRBuilder/opcode table requirements and byte-identity parity expectations.
  • Clarifies purity behavior as an allowlist (PURE_OP_NAMES) with a C++ mirror (rocke_opcode_pure[]).
  • Updates stale example path references from example/ck_tile/dsl/... to python/rocke/examples/....
Suppressed comments (1)

dnn-providers/hip-kernel-provider/rocke/platform/dsl_docs/development/extending.md:289

  • Similarly, this parity-example instruction still points to a python/rocke/examples/<NN>_my_op/gen.py generator script. The existing python/rocke/examples/ layout is module-based (e.g. common/*.py) and uses run_all.py + _goldens/ rather than per-example gen.py entrypoints. Updating this reference to the current examples registration mechanism would avoid sending contributors to create files in a pattern that the repo doesn’t currently use.
### 3.9 Optional CK Tile parity example

For end-to-end gating, add `python/rocke/examples/<NN>_my_op/gen.py`:


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@yraparti

yraparti commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

LGTM with minor nits.
Section 1 is a clear, accurate improvement. Section 3 path updates are directionally better but still don’t match the tree.


What’s good

Fills a real gap. The old doc stopped at Python core/ir.py and LLVM lowering. The new §1.2 covers the C++ side authors actually need: rocke_b_<op>, rocke_opcode_t, and the three parallel tables in core_types.cpp / verify.cpp.

Purity correction is accurate. The old text implied side-effecting ops were inferred by default. The new allowlist model matches the code:

    def is_pure(self) -> bool:
        if "pure" in self.attrs:
            return bool(self.attrs["pure"])
        return is_pure_op_name(self.name)

Opcode-table warning is well grounded. core_types.cpp documents enum-order coupling, and future_intrinsic_lowering.cpp has case_opcode_names_are_aligned for the same failure mode. Calling out silent mis-indexing is the right emphasis.

Seven-touchpoint checklist gives a practical ordering: Python builder → C++ builder/tables → dual LLVM lowerers → optional HIP → serialization → tests/byte-identity.

Examples and API references check out: rocke_i_unop / rocke_i_binop, rocke_b_op, rocke_i_live(b), ROCKE_OP_MATH_* naming, and the C++ lowering paths under cpp/core/lower_llvm/.


Nits / suggestions

1. §1.2 cross-references §1.4 for byte-identity

Two places say “byte-identity differential (§1.4)” when talking about C++ IRBuilder authoring. §1.4 is LLVM lowering; byte-identity is §1.6 / touchpoint 7. §1.4 does mention byte-identity, but the C++-builder context is really “both engines must author the op before lowering can be compared.” Point to §1.6 (or touchpoint 7) instead.

2. Section 3 example paths are still stale

The PR updates example/ck_tile/dsl/... → python/rocke/examples/<NN>_my_op/..., but on this branch:

  • There is no test_rocke_examples.py under platform/
  • There are no gen.py files under python/rocke/examples/
  • Live examples use arch/workload layout (python/rocke/examples/gfx950/..., common/...), per authoring_model.md

dsl_docs/examples/index.md still describes two forms (Python-owned scripts vs CMake gen.py + expected.json). Section 3.9’s gen.py / expected.json workflow doesn’t match a flat python/rocke/examples/<NN>_my_op/ path.

Either keep this PR scoped to §1 (strongest part), or align §3 with the real layout and note whether gen.py parity examples still exist / are planned.

3. Optional: mention the opcode alignment test

Worth a one-liner that tests/core/future_intrinsic_lowering.cpp::case_opcode_names_are_aligned catches table drift even when byte-identity families don’t exercise the new op.

4. Optional: Python textual printer

§1.3 (ir_print.py) is still optional and easy to skip. A brief “also mirror in C++ printer if you care about debug output” note would complete the dual-engine story.


Accuracy spot-check (code vs doc)

Doc claim Verified
PURE_OP_NAMES allowlist + C++ rocke_opcode_pure[] Yes
rocke_opcode_names[] must match Python op.name Yes (comment in core_types.cpp)
verify.cpp arity classifier groups Yes (is_unary_same_type, binop cases)
Serialization is generic / opcode-name driven Yes (serialize.cpp / ir_serialize.py)
python/rocke/examples/<NN>_my_op/gen.py for parity No — path/workflow not present in tree

Recommendation: Merge for §1. Consider a follow-up (or a small edit in this PR) for the §1.2 cross-ref and §3 example-path accuracy.

@yraparti yraparti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

address nit in a different ticket.

@CongMa13
CongMa13 merged commit 5e5597d into develop Sep 14, 2026
35 of 36 checks passed
@CongMa13
CongMa13 deleted the users/congma/ck/aick-1711-add-ir-op-docs branch September 14, 2026 20:56
@CongMa13

Copy link
Copy Markdown
Contributor Author

Will address nit in AICK-2246

@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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.

5 participants