Repository navigation
[rocjitsu] Fix ds_read_b64_tr_b16 lane layout in gfx950 simulator #10175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2511,6 +2511,7 @@ if(RJ_ENABLE_HIP_TESTS) | |
| # generic HIP smoke tests are built for a local RDNA host such as | ||
| # gfx1201. | ||
| rj_add_hip_test(hip_ds_transpose_acc_test ARCH gfx950) | ||
| rj_add_hip_test(hip_ds_read_tr_b16_sim_test ARCH gfx950) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please also register this test with ctest. rj_add_hip_test only creates the build target. eg: |
||
|
|
||
| rj_add_hip_test( | ||
| cdna4_to_cdna3_lds_hip_test | ||
|
|
||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| // Copyright (c) 2026 Advanced Micro Devices, Inc. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| /// @file hip_ds_read_tr_b16_sim_test.cpp | ||
| /// @brief Verify the ds_read_b64_tr_b16 cross-lane layout on gfx950. | ||
| /// | ||
| /// One wave writes a known per-lane 4-halfword pattern to LDS with | ||
| /// ds_write_b64, reads it back with ds_read_b64_tr_b16, and checks every lane | ||
| /// against the 4x16-lane halfword transpose: destination lane l halfword n | ||
| /// comes from source lane ((l & 0x30) | ((l & 0xc) >> 2)) + 4 * n, halfword | ||
| /// (l & 3). | ||
|
|
||
| #include <cstdint> | ||
| #include <hip/hip_runtime.h> | ||
| #include <vector> | ||
|
|
||
| #include <gtest/gtest.h> | ||
|
|
||
| int main(int argc, char **argv) { | ||
| ::testing::InitGoogleTest(&argc, argv); | ||
| int rc = RUN_ALL_TESTS(); | ||
| (void)hipDeviceReset(); | ||
| return rc; | ||
| } | ||
|
|
||
| #define HIP_ASSERT(call) \ | ||
| do { \ | ||
| hipError_t err = (call); \ | ||
| ASSERT_EQ(err, hipSuccess) << "HIP error: " << hipGetErrorString(err); \ | ||
| } while (0) | ||
|
|
||
| namespace { | ||
|
|
||
| constexpr int kWaveSize = 64; | ||
|
|
||
| using U32x2 = uint32_t __attribute__((ext_vector_type(2))); | ||
|
|
||
| constexpr uint32_t tr_b16_halfword_value(int thread, int halfword) { | ||
| return (0x1200u + static_cast<uint32_t>(thread) * 0x11u + static_cast<uint32_t>(halfword)) & | ||
| 0xffffu; | ||
| } | ||
|
|
||
| constexpr uint32_t pack_u16_pair(uint32_t lo, uint32_t hi) { | ||
| return (lo & 0xffffu) | ((hi & 0xffffu) << 16); | ||
| } | ||
|
|
||
| __global__ __launch_bounds__(kWaveSize) void ds_read_tr_b16_kernel(uint32_t *out) { | ||
| __shared__ uint32_t lds[kWaveSize * 2]; | ||
| const int tid = threadIdx.x; | ||
| const uint32_t addr = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(lds)) + tid * 8; | ||
| U32x2 raw = {pack_u16_pair(tr_b16_halfword_value(tid, 0), tr_b16_halfword_value(tid, 1)), | ||
| pack_u16_pair(tr_b16_halfword_value(tid, 2), tr_b16_halfword_value(tid, 3))}; | ||
| U32x2 value = {}; | ||
| asm volatile("ds_write_b64 %0, %1\n" | ||
| "s_waitcnt lgkmcnt(0)\n" | ||
| : | ||
| : "v"(addr), "v"(raw) | ||
| : "memory"); | ||
| __syncthreads(); | ||
| asm volatile("ds_read_b64_tr_b16 %0, %1\n" | ||
| "s_waitcnt lgkmcnt(0)\n" | ||
| : "=v"(value) | ||
| : "v"(addr) | ||
| : "memory"); | ||
| out[tid * 2 + 0] = value[0]; | ||
| out[tid * 2 + 1] = value[1]; | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| TEST(DsReadTrB16SimTest, LaneLayout) { | ||
| constexpr int kOutWords = kWaveSize * 2; | ||
| uint32_t *device_out = nullptr; | ||
| HIP_ASSERT(hipMalloc(&device_out, kOutWords * sizeof(uint32_t))); | ||
| HIP_ASSERT(hipMemset(device_out, 0, kOutWords * sizeof(uint32_t))); | ||
|
|
||
| ds_read_tr_b16_kernel<<<1, kWaveSize>>>(device_out); | ||
| HIP_ASSERT(hipDeviceSynchronize()); | ||
|
|
||
| std::vector<uint32_t> host_out(kOutWords); | ||
| HIP_ASSERT( | ||
| hipMemcpy(host_out.data(), device_out, kOutWords * sizeof(uint32_t), hipMemcpyDeviceToHost)); | ||
| (void)hipFree(device_out); | ||
|
|
||
| for (int lane = 0; lane < kWaveSize; ++lane) { | ||
| const int halfword = lane & 3; | ||
| const int source_base = (lane & 0x30) + ((lane & 0x0c) >> 2); | ||
| const uint32_t expected_lo = pack_u16_pair(tr_b16_halfword_value(source_base + 0, halfword), | ||
| tr_b16_halfword_value(source_base + 4, halfword)); | ||
| const uint32_t expected_hi = pack_u16_pair(tr_b16_halfword_value(source_base + 8, halfword), | ||
| tr_b16_halfword_value(source_base + 12, halfword)); | ||
| EXPECT_EQ(host_out[lane * 2 + 0], expected_lo) << "lane " << lane << " low dword"; | ||
| EXPECT_EQ(host_out[lane * 2 + 1], expected_hi) << "lane " << lane << " high dword"; | ||
| } | ||
| } |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we preserve this layout distinction in InstructionSemantics instead of inferring it here from read width and wave size?
TransposeKind::TR_B16is now both CDNA4 ds_read_b64_tr_b16 and B128 TR16 loads. Since these are distinct layouts, please split them into separate transpose/layout kinds and dispatch on that here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SG, let me refactor this part. thanks for the reivew!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in a466e5a — split into
TR16_B128(id 4, gfx1250/RDNA4 B128 loads, unchanged) and a newB64_TR_B16(id 5, CDNA4ds_read_b64_tr_b16), emitted fromInstructionSemantics;transpose_response()now dispatches on the kind. Gave the new id to the CDNA4 form so onlycdna4/ds_exec.cppneeded regenerating.