Skip to content

Conversation

rampitec
Copy link
Collaborator

The pattern does not factor saddr. There is no way to write a test
for it because gfx1200 does not have sram-ecc but also no saddr,
and gfx1250 does not fall into this preserving category while has
sram-ecc. Nevertheless, the day we could fix it that would become a
problem. For now it is OK that change does not fail.

That was untested before and it is untested now, but at least t16
block uses t16 patterns.

The pattern does not factor saddr. There is no way to write a test
for it because gfx1200 does not have sram-ecc but also no saddr,
and gfx1250 does not fall into this preserving category while has
sram-ecc. Nevertheless, the day we could fix it that would become a
problem. For now it is OK that change does not fail.

That was untested before and it is untested now, but at least t16
block uses t16 patterns.
Copy link
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@rampitec rampitec marked this pull request as ready for review September 23, 2025 08:01
@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Stanislav Mekhanoshin (rampitec)

Changes

The pattern does not factor saddr. There is no way to write a test
for it because gfx1200 does not have sram-ecc but also no saddr,
and gfx1250 does not fall into this preserving category while has
sram-ecc. Nevertheless, the day we could fix it that would become a
problem. For now it is OK that change does not fail.

That was untested before and it is untested now, but at least t16
block uses t16 patterns.


Full diff: https://github.com/llvm/llvm-project/pull/160253.diff

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/FLATInstructions.td (+2-2)
diff --git a/llvm/lib/Target/AMDGPU/FLATInstructions.td b/llvm/lib/Target/AMDGPU/FLATInstructions.td
index 7e5ae25ff30e6..9f33bac4c56ea 100644
--- a/llvm/lib/Target/AMDGPU/FLATInstructions.td
+++ b/llvm/lib/Target/AMDGPU/FLATInstructions.td
@@ -2009,8 +2009,8 @@ let OtherPredicates = [D16PreservesUnusedBits, HasFlatAddressSpace], True16Predi
   defm : FlatLoadPats_D16_t16<FLAT_LOAD_SBYTE_D16_t16, atomic_load_sext_8_flat, i16>;
   defm : FlatStorePats_t16 <FLAT_STORE_BYTE, truncstorei8_flat, i16>;
   defm : FlatStorePats_t16 <FLAT_STORE_SHORT, store_flat, i16>;
-  def : FlatStorePat <FLAT_STORE_BYTE_t16, atomic_store_8_flat, i16>;
-  def : FlatStorePat <FLAT_STORE_SHORT_t16, atomic_store_16_flat, i16>;
+  defm : FlatStorePats_t16 <FLAT_STORE_BYTE, atomic_store_8_flat, i16>;
+  defm : FlatStorePats_t16 <FLAT_STORE_SHORT, atomic_store_16_flat, i16>;
 } // End let OtherPredicates = [D16PreservesUnusedBits, HasFlatAddressSpace], True16Predicate = UseRealTrue16Insts
 
 defm : FlatLoadPats <FLAT_LOAD_DWORD, atomic_load_nonext_32_flat, i32>;

@rampitec rampitec merged commit e45993d into main Sep 23, 2025
13 checks passed
@rampitec rampitec deleted the users/rampitec/09-23-_amdgpu_fix_sub-dword_atomic_flat_saddr_store_with_no_d16._nfci branch September 23, 2025 16:44
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.

3 participants