Skip to content

[rocjitsu] Regenerate matrix conversion header during DBT codegen - #13050

Merged
kuhar merged 4 commits into
developfrom
users/newling/fix-rocjitsu-matrix-regeneration
Oct 19, 2026
Merged

kuhar merged 4 commits into
developfrom
users/newling/fix-rocjitsu-matrix-regeneration

Conversation

@newling

@newling newling commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

matrix_conversions.h supplies the lane-permutation data used when translating CDNA4 MFMA instructions to RDNA4 WMMA instructions. A report that emulation/rocjitsu/scripts/generate-amdisa.sh could not recreate this header exposed a missing generation step: the Python layout derivation and header emitter existed, but python -m amdisa never called them. DBT generation therefore succeeded while leaving a missing header absent or an existing header untouched, even if its contents were stale. The checked-in copy hid this defect in populated checkouts. The header and emitter were introduced in #5942.

Call the existing derivation and emitter during DBT generation so both --dbt-output and the --isa-output fallback recreate the header. An explicit inventory preserves the four existing source/target pairs and every table value; these pairs describe lane layouts, and adding one does not establish instruction availability or implement a new translation. Document the inventory and layout formulas, and update the CLI help to describe the additional output. Also emit <cstddef> and use std::size_t, fixing the header's failure to compile without preceding includes.

CLI regressions cover missing and stale headers with each output option and compare the complete regenerated catalog against the tracked header. The focused generator, rule and layout tests pass on the rebased branch, as do standalone C++20 compilation with assertions for all four original entries and changed-file pre-commit hooks. Two existing tests skip because optional semantics XML is unavailable.

@github-actions github-actions Bot added documentation Improvements or additions to documentation project: rocjitsu labels Oct 7, 2026
@therock-pr-bot

therock-pr-bot Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 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

therock-pr-bot Bot commented Oct 7, 2026

Copy link
Copy Markdown

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

@newling
newling force-pushed the users/newling/fix-rocjitsu-matrix-regeneration branch from 7094409 to d8805c0 Compare October 13, 2026 21:18
@newling
newling marked this pull request as ready for review October 13, 2026 21:19
@newling
newling requested review from a team and atgutier as code owners October 13, 2026 21:19

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would help to tidy two misleading parts of the emitter's interface and attribution while wiring it into the CLI:

  • emulation/rocjitsu/lib/python/amdisa/codegen/dbt/rule_gen.py:316: remove the unused guard parameter. Passing a custom value has no effect because the emitter always uses #pragma once, and no in-tree caller supplies it.
  • emulation/rocjitsu/lib/python/amdisa/codegen/dbt/rule_gen.py:325: update the generated banner to name rule_gen.py or the amdisa codegen pipeline. It currently points maintainers to layout_catalog, which derives layouts but does not emit the header.

These are small clarity improvements; neither changes the generated table values.

Comment thread emulation/rocjitsu/lib/python/amdisa/codegen/dbt/rule_gen.py Outdated
Comment thread emulation/rocjitsu/lib/python/amdisa/__main__.py Outdated
Comment thread emulation/rocjitsu/lib/python/amdisa/tests/test_matrix_conversions_codegen.py Outdated

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added 2 comments

Comment thread emulation/rocjitsu/lib/python/amdisa/codegen/dbt/rule_gen.py Outdated
Comment thread emulation/rocjitsu/docs/sphinx/how-to/regenerate-isa-codegen.md
@newling
newling force-pushed the users/newling/fix-rocjitsu-matrix-regeneration branch from d8805c0 to 164ddf1 Compare October 15, 2026 17:21
@newling
newling requested a review from kuhar October 15, 2026 19:18

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment thread emulation/rocjitsu/lib/python/amdisa/tests/test_matrix_conversions_codegen.py Outdated
@newling
newling force-pushed the users/newling/fix-rocjitsu-matrix-regeneration branch 4 times, most recently from 3447f1f to 42d3f2e Compare October 18, 2026 17:24
@newling
newling force-pushed the users/newling/fix-rocjitsu-matrix-regeneration branch from 42d3f2e to d7f567c Compare October 18, 2026 17:54
@kuhar
kuhar merged commit 55754a0 into develop Oct 19, 2026
72 of 74 checks passed
@kuhar
kuhar deleted the users/newling/fix-rocjitsu-matrix-regeneration branch October 19, 2026 00:01
@systems-assistant

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

Labels

documentation Improvements or additions to documentation organization: ROCm project: rocjitsu

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants