Repository navigation
[rocjitsu] Model gfx1250 SRAM ECC semantics for D16 loads - #10445
Merged
Merged
Conversation
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
kuhar
reviewed
Aug 20, 2026
newling
force-pushed
the
fix/gfx1250-sram-ecc-d16-tail-loads
branch
from
August 20, 2026 14:56
23e99ab to
5c22619
Compare
Contributor
Author
|
Failure in tsan looks like an intermittent unrelated to this PR. I need to rebase anyway to resolve conflicts, so am going to just do that. btw @kuhar I think I've fully addressed your comments. I would also like to confirm we have the correct setting the architectures other than cdna5, but will do that as a follow up. |
newling
force-pushed
the
fix/gfx1250-sram-ecc-d16-tail-loads
branch
from
August 20, 2026 15:24
5c22619 to
4d9fe48
Compare
kuhar
approved these changes
Aug 20, 2026
kuhar
left a comment
kuhar
left a comment
Member
There was a problem hiding this comment.
LGTM % one reply here #10445 (comment)
newling
force-pushed
the
fix/gfx1250-sram-ecc-d16-tail-loads
branch
from
August 20, 2026 16:51
4d9fe48 to
47794b7
Compare
newling
force-pushed
the
fix/gfx1250-sram-ecc-d16-tail-loads
branch
from
August 20, 2026 17:10
47794b7 to
7e2bd6f
Compare
cmcknigh
pushed a commit
that referenced
this pull request
Aug 25, 2026
gfx1250 does support SRAM ECC: - LLVM's `GFX1250` processor model enables `FEATURE_SRAMECC`: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/GCNProcessors.td - The AMD Instinct CDNA5 ISA specifies that, on chips with ECC, D16 and D16_HI loads write the full 32-bit VGPR and fill the unused portion with zero: https://docs.amd.com/api/khub/documents/YblmXrYRcxw2GO7d0No8Lw/content Correcting this in rocJITsu fixes a numerical failure in an i8 hipBLASLt GEMM. All of the agent-generated details, including the exact workload, provenance, commands, deterministic before/after results, and instruction-level regression, can be found here: https://gist.github.com/newling/038f6eb6fc9dfb9c53437a5791a26301 ## Issue Tracking Related: #9935
ajanicijamd
pushed a commit
that referenced
this pull request
Aug 25, 2026
gfx1250 does support SRAM ECC: - LLVM's `GFX1250` processor model enables `FEATURE_SRAMECC`: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/GCNProcessors.td - The AMD Instinct CDNA5 ISA specifies that, on chips with ECC, D16 and D16_HI loads write the full 32-bit VGPR and fill the unused portion with zero: https://docs.amd.com/api/khub/documents/YblmXrYRcxw2GO7d0No8Lw/content Correcting this in rocJITsu fixes a numerical failure in an i8 hipBLASLt GEMM. All of the agent-generated details, including the exact workload, provenance, commands, deterministic before/after results, and instruction-level regression, can be found here: https://gist.github.com/newling/038f6eb6fc9dfb9c53437a5791a26301 ## Issue Tracking Related: #9935
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
gfx1250 does support SRAM ECC:
GFX1250processor model enablesFEATURE_SRAMECC:https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/GCNProcessors.td
D16_HI loads write the full 32-bit VGPR and fill the unused portion with
zero:
https://docs.amd.com/api/khub/documents/YblmXrYRcxw2GO7d0No8Lw/content
Correcting this in rocJITsu fixes a numerical failure in an i8 hipBLASLt
GEMM.
All of the agent-generated details, including the exact workload, provenance,
commands, deterministic before/after results, and instruction-level
regression, can be found here:
https://gist.github.com/newling/038f6eb6fc9dfb9c53437a5791a26301
Issue Tracking
Related: #9935