Skip to content

[rocjitsu] Compact generated operand storage - #14779

Merged
kuhar merged 1 commit into
developfrom
users/kuhar/perf-operand-packing
Nov 3, 2026
Merged

kuhar merged 1 commit into
developfrom
users/kuhar/perf-operand-packing

Conversation

@kuhar

@kuhar kuhar commented Nov 2, 2026

Copy link
Copy Markdown
Member

Use one byte for literal widening and group generated operand fields by
alignment across all ten AMDGPU architectures.

On x86-64, shrink generated operands from 80 to 64 bytes on all ten
targets. For CDNA5, a scalar add with carry drops from 536 to 472 bytes,
a VOP3 move from 400 to 368 bytes, and VOPD from 992 to 864 bytes.

Measure warm userspace CPU counters on a Threadripper PRO 9995WX with
GCC 16 Release/O3 and x86-64-v4, using one simulation worker on one CPU:

  • Eleven distinct ML inputs, two paired sweeps: cycles -0.67%, L2
    data-request misses -3.09%, instructions -0.06%.
  • Seven small kernels on CDNA3/4/5, three paired sweeps: cycles -0.21%.
    By target: CDNA3 -0.11%, CDNA4 +0.65%, CDNA5 -1.16%.

Cycles vary by target despite the same operand size reduction: CDNA5 ML
cycles decrease 1.59%, while CDNA4 increases 0.47%. These measurements
do not isolate the cause of that difference. Use equal-cell geometric
means. Measurements are unreserved, with CPU affinity, a 2.5 GHz frequency
policy, ASLR disabled, wait checking enabled and an exclusive smoke flock.

Use one byte for literal widening and group generated operand fields by
alignment across all ten AMDGPU architectures.

On x86-64, shrink generated operands from 80 to 64 bytes on all ten
targets. For CDNA5, a scalar add with carry drops from 536 to 472 bytes,
a VOP3 move from 400 to 368 bytes, and VOPD from 992 to 864 bytes.

Measure warm userspace CPU counters on a Threadripper PRO 9995WX with
GCC 16 Release/O3 and x86-64-v4, using one simulation worker on one CPU:

- Eleven distinct ML inputs, two paired sweeps: cycles -0.67%, L2
  data-request misses -3.09%, instructions -0.06%.
- Seven small kernels on CDNA3/4/5, three paired sweeps: cycles -0.21%.
  By target: CDNA3 -0.11%, CDNA4 +0.65%, CDNA5 -1.16%.

Cycles vary by target despite the same operand size reduction: CDNA5 ML
cycles decrease 1.59%, while CDNA4 increases 0.47%. These measurements
do not isolate the cause of that difference. Use equal-cell geometric
means. Measurements are unreserved, with CPU affinity, a 2.5 GHz frequency
policy, ASLR disabled, wait checking enabled and an exclusive smoke flock.
@kuhar
kuhar requested review from a team and atgutier as code owners November 2, 2026 02:54
Copilot AI balanced review requested due to automatic review settings November 2, 2026 02:54
@therock-pr-bot

therock-pr-bot Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• JIRA ID (on separate line)
ROCM-25757
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID (on separate line)
AIRUNTIME-2352
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
• Closes #10
• Fixes octo-org/octo-repo#100
• Resolves: #123
• #123
• https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 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 Nov 2, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

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.

🟢 Approval recommended

No active review issues were identified for the operand-storage compaction.

No open issues 🎉

What changed in this PR

This PR reduces rocjitsu’s generated AMDGPU operand storage from 80 to 64 bytes on x86-64 across all ten architectures, reducing the size of instruction objects that contain operands.

Changes:

  • Uses uint8_t for literal-widening enums and reorders fields to reduce alignment padding.
  • Updates the generator and all ten generated operand headers consistently.
  • Updates layout assertions for representative operand and instruction types, including VOPD.
File Description
emulation/​rocjitsu/​tests/​decode_smoke_test.cpp Checks compact operand and instruction sizes.
emulation/​rocjitsu/​lib/​rocjitsu/​src/​rocjitsu/​isa/​arch/​amdgpu/​generated/​rdna4/​operand.h Compacts RDNA4 operand storage.
emulation/​rocjitsu/​lib/​rocjitsu/​src/​rocjitsu/​isa/​arch/​amdgpu/​generated/​rdna3/​operand.h Compacts RDNA3 operand storage.
emulation/​rocjitsu/​lib/​rocjitsu/​src/​rocjitsu/​isa/​arch/​amdgpu/​generated/​rdna3_5/​operand.h Compacts RDNA3.5 operand storage.
emulation/​rocjitsu/​lib/​rocjitsu/​src/​rocjitsu/​isa/​arch/​amdgpu/​generated/​rdna2/​operand.h Compacts RDNA2 operand storage.
emulation/​rocjitsu/​lib/​rocjitsu/​src/​rocjitsu/​isa/​arch/​amdgpu/​generated/​rdna1/​operand.h Compacts RDNA1 operand storage.
emulation/​rocjitsu/​lib/​rocjitsu/​src/​rocjitsu/​isa/​arch/​amdgpu/​generated/​cdna5/​operand.h Compacts CDNA5 operand storage.
emulation/​rocjitsu/​lib/​rocjitsu/​src/​rocjitsu/​isa/​arch/​amdgpu/​generated/​cdna4/​operand.h Compacts CDNA4 operand storage.
emulation/​rocjitsu/​lib/​rocjitsu/​src/​rocjitsu/​isa/​arch/​amdgpu/​generated/​cdna3/​operand.h Compacts CDNA3 operand storage.
emulation/​rocjitsu/​lib/​rocjitsu/​src/​rocjitsu/​isa/​arch/​amdgpu/​generated/​cdna2/​operand.h Compacts CDNA2 operand storage.
emulation/​rocjitsu/​lib/​rocjitsu/​src/​rocjitsu/​isa/​arch/​amdgpu/​generated/​cdna1/​operand.h Compacts CDNA1 operand storage.
emulation/​rocjitsu/​lib/​python/​amdisa/​codegen/​_generator.py Emits the compact enum and field layout.

Review effort: Balanced

@IanWood1 IanWood1 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!

' uint64_t widened_literal32_value() const;\n'
' // Pack literal state with the byte-sized widening enum to keep operands at 64 bytes.\n'
' uint64_t literal64_value_ = 0;\n'
' std::optional<Literal32Widening> literal32_widening_;\n'

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.

Moving this field before execution_backend_ lets it use part of the four bytes of tail padding in the base class (IsaOperand<Isa>), leaving room at the end for future flags without increasing the operand size.

@kuhar
kuhar merged commit 2b3c002 into develop Nov 3, 2026
139 of 148 checks passed
@kuhar
kuhar deleted the users/kuhar/perf-operand-packing branch November 3, 2026 18:29
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

rocjitsu post-commit fuzz passed

The 32-worker multiplexer campaigns for commit 2b3c00291b7d completed without recording a crash or hang.

  • ASan+UBSan (6-minute): 644,826 input executions (1,828.32 inputs/s)
  • TSan (6-minute): 522,159 input executions (1,490.95 inputs/s)

View the workflow run.

@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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants