Skip to content

Revert correcting the VGPR size for GFX 11.5.1 - #2268

Merged
superm1 merged 1 commit into
ROCm:developfrom
superm1:revert-vgpr-strix-halo
Dec 11, 2025
Merged

superm1 merged 1 commit into
ROCm:developfrom
superm1:revert-vgpr-strix-halo

Conversation

@superm1

@superm1 superm1 commented Dec 11, 2025

Copy link
Copy Markdown
Contributor

Although the value is correct; there is no source of truth between kernel and userspace. This leads to problems if the kernel has strict restrictions (such as kernel 6.17 or earlier). The restrictions were lifted in 6.17.9 and and 6.18, but there is no guarantee userspace is using this.

So short term this value will be wrong. But on newer kernels the kernel will communicate the right size and rocr-runtime will be adjusted to use that.

Link: ROCm/TheRock#2505

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

This PR reverts a previous correction to the VGPR size for GFX 11.5.1 to address kernel/userspace compatibility issues. While the previously corrected value (0x60000) was technically correct, it causes problems with kernels prior to 6.17.9 and 6.18 that have strict restrictions. The revert allows GFX 11.5.1 to fall back to the default VGPR size (0x40000) temporarily, until newer kernels can communicate the correct size directly.

  • Removes GFX_VERSION_GFX1151 from the condition that sets VGPR size to 0x60000, allowing it to use the default 0x40000 instead

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jayhawk-commits

Copy link
Copy Markdown
Collaborator

@jayhawk-commits

Copy link
Copy Markdown
Collaborator

You can explicitly delete the patch from TheRock in these workflows, by adding a new line with the specific file name.

https://github.com/superm1/rocm-systems/blob/revert-vgpr-strix-halo/.github/workflows/therock-ci-linux.yml#L68 https://github.com/superm1/rocm-systems/blob/revert-vgpr-strix-halo/.github/workflows/therock-ci-windows.yml#L56

By including the changes to these workflows in this same PR, the chicken-egg problem is dealt with.

@superm1
superm1 force-pushed the revert-vgpr-strix-halo branch from 06940ae to 77497f7 Compare December 11, 2025 02:56
@superm1
superm1 requested a review from a team as a code owner December 11, 2025 02:56
@github-actions github-actions Bot added the github actions Pull requests that update GitHub Actions code label Dec 11, 2025

@jayhawk-commits jayhawk-commits left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

Although the value is correct; there is no source of truth between
kernel and userspace.  This leads to problems if the kernel has strict
restrictions (such as kernel 6.17 or earlier). The restrictions were
lifted in 6.17.9 and and 6.18, but there is no guarantee userspace is
using this.

So short term this value will be wrong.  But on newer kernels the kernel
will communicate the right size and rocr-runtime will be adjusted to
use that.

Link: ROCm/TheRock#2505
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
@superm1
superm1 force-pushed the revert-vgpr-strix-halo branch from 77497f7 to dd3f03f Compare December 11, 2025 05:23
@superm1
superm1 merged commit 0c4d08f into ROCm:develop Dec 11, 2025
21 checks passed
systems-assistant Bot pushed a commit to ROCm/ROCR-Runtime that referenced this pull request Dec 11, 2025
Although the value is correct; there is no source of truth between
kernel and userspace.  This leads to problems if the kernel has strict
restrictions (such as kernel 6.17 or earlier). The restrictions were
lifted in 6.17.9 and and 6.18, but there is no guarantee userspace is
using this.

So short term this value will be wrong.  But on newer kernels the kernel
will communicate the right size and rocr-runtime will be adjusted to
use that.

Link: ROCm/TheRock#2505

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
[rocm-systems] ROCm/rocm-systems#2268 (commit 0c4d08f)
xuchen-amd pushed a commit that referenced this pull request Dec 16, 2025
Although the value is correct; there is no source of truth between
kernel and userspace.  This leads to problems if the kernel has strict
restrictions (such as kernel 6.17 or earlier). The restrictions were
lifted in 6.17.9 and and 6.18, but there is no guarantee userspace is
using this.

So short term this value will be wrong.  But on newer kernels the kernel
will communicate the right size and rocr-runtime will be adjusted to
use that.

Link: ROCm/TheRock#2505

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
jayhawk-commits added a commit to ROCm/TheRock that referenced this pull request Dec 19, 2025
- Bump `rocm-systems` 20251218
- Drop `0002-Revert-hsakmt-bump-vgpr-count-for-gfx1151-1807.patch`
mapped to ROCm/rocm-systems#2268
- Addresses #2642 thanks to ROCm/rocm-systems#2399
benrichard-amd pushed a commit to benrichard-amd/TheRock that referenced this pull request Jan 5, 2026
- Bump `rocm-systems` 20251218
- Drop `0002-Revert-hsakmt-bump-vgpr-count-for-gfx1151-1807.patch`
mapped to ROCm/rocm-systems#2268
- Addresses ROCm#2642 thanks to ROCm/rocm-systems#2399
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github actions Pull requests that update GitHub Actions code organization: ROCm project: rocr-runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants