Skip to content

Pytorch/rocm7.1 internal testing hstu drop 1#881

Open
scxiao wants to merge 21 commits into
pytorch/rocm7.1_internal_testingfrom
pytorch/rocm7.1_internal_testing_hstu_drop_1
Open

Pytorch/rocm7.1 internal testing hstu drop 1#881
scxiao wants to merge 21 commits into
pytorch/rocm7.1_internal_testingfrom
pytorch/rocm7.1_internal_testing_hstu_drop_1

Conversation

@scxiao
Copy link
Copy Markdown

@scxiao scxiao commented Sep 23, 2025

The core Triton is a small number of people, and we receive many PRs (thank
you!). To help us review your code more quickly, if you are a new
contributor (less than 3 PRs merged) we ask that you complete the following
tasks and include the filled-out checklist in your PR description.

This PR is to force to turn on buffer ops for triton kernels.

Complete the following tasks before sending your PR, and replace [ ] with
[x] to indicate you have done them.

  • I am not making a trivial change, such as fixing a typo in a comment.

  • I have written a PR description following these
    rules.

  • I have run pre-commit run --from-ref origin/main --to-ref HEAD.

  • Select one of the following.

    • I have added tests.
      • /test for lit tests
      • /unittest for C++ tests
      • /python/test for end-to-end tests
    • This PR does not need a test because FILL THIS IN.
  • Select one of the following.

    • I have not added any lit tests.
    • The lit tests I have added follow these best practices,
      including the "tests should be minimal" section. (Usually running Python code
      and using the instructions it generates is not minimal.)

scxiao and others added 19 commits September 22, 2025 20:30
This PR generalize flags in translate_to_asm function and
adds new variant to HIPOptions.schedule_hint for iterative-ilp scheduler.
See details here

triton-lang#8145

---------

Co-authored-by: Vinayak Gokhale <Vinayak.Gokhale@amd.com>
Co-authored-by: Dewei Wang <Dewei.Wang@amd.com>
)

On `GFX9`, padding alone is not enough to avoid shared memory bank
conflicts when using direct-to-LDS loads.
For example, a `global_load_lds_dwordx4` instruction loads `64 threads ×
16 bytes = 1024 bytes` at once, so padding can only be inserted at
1024-byte boundaries in LDS.

If the tensor’s rows are smaller than the transfer size, we can’t insert
padding between them, since they are loaded by the same instruction and
written contiguously to shared memory.
For instance, if each row is 256 bytes, the layout would look like:
```
[[row0], [row1], [row2], [row3], [padding], [row4], [row5], [row6], [row7], [pading], [row8] ...]
```
However, to avoid bank conflicts when reading the data, we would
actually want padding between row0 and row1, to shift elements from the
same column to different banks.

To solve this, we need to rearrange the rows in the shared memory layout
so that padding separates contiguous rows of the tensor. In this
example, by padding after every 4 rows, we could permute rows like this:

```
[[row0], [row16], [row32], [row48], [padding] [row1], [row17], [row33], [row49], [padding], [row2], [row18] ...]
```
This allows us to load the data conflict free.

This PR adds an optional linear component to `PaddedSharedEncoding` that
allows us to permute the rows in LDS.

There will be follow-up PRs adding support for them in the stream
pipeliner and when lowering direct-to-lds loads on AMD.
Stop requiring local_load to have a DotOperand layout attached when
trying to generate ds_read_tr instructions.
This simplifies the code and allows loading of more complex cases such
as local_load + tt.trans.
)

This is a very old bug. The problem can be explained by following simple
statement, where the fatPtrs is a simple wrapper of a hashmap.

  fatPtrs[toKey] = fatPtrs.at(fromKey)

time 0: RHS's at() function return a reference to an existing element.
time 1: There is no element associated with toKey, so the LHS needs to
resize its internal bucket in order to accommodate the new entry. When
resize, it invalidates the reference it took in the step 1

The fix is to change the map's value type from an aggregate type ValueT
to std::unique_ptr<ValueT>. With this change, when map resizes, while
unique_ptr may change, the raw pointer it maintains is unchanged, and
hence we can get a "sticky" pointer for the data it maps to.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants