for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) {
          if (gpuDynInst->exec_mask[lane]) {

Matthew Poremba has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/70577?usp=email ) Change subject: arch-gcn3,arch-vega: Fix ds_read2st64_b32 ...................................................................... arch-gcn3,arch-vega: Fix ds_read2st64_b32 This instruction has two issues. The first is that it should write two consecutive registers, starting with vdst because it is writing two dwords. The second is that the data assignment to the lanes from the dynamic instruction should cast to a U32 type otherwise the array index goes out of bounds and returns the wrong data. The first issue was fixed in GCN3 a few years ago in this review: https://gem5-review.googlesource.com/c/public/gem5/+/32236. This changeset makes the same change for Vega and applies the U32 cast in both ISAs. Tested with rocPRIM unit test. The test was failing before this changeset and now passes. Change-Id: Ifb110fc9a36ad198da7eaf86b1e3e37eccd3bb10 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/70577 Maintainer: Matt Sinclair <mattdsinclair@gmail.com> Reviewed-by: Matt Sinclair <mattdsinclair@gmail.com> Tested-by: kokoro <noreply+kokoro@google.com> --- M src/arch/amdgpu/gcn3/insts/instructions.cc M src/arch/amdgpu/vega/insts/instructions.cc 2 files changed, 5 insertions(+), 5 deletions(-) Approvals: kokoro: Regressions pass Matt Sinclair: Looks good to me, approved; Looks good to me, approved diff --git a/src/arch/amdgpu/gcn3/insts/instructions.cc b/src/arch/amdgpu/gcn3/insts/instructions.cc index 8c51af5..478b1d3 100644 --- a/src/arch/amdgpu/gcn3/insts/instructions.cc +++ b/src/arch/amdgpu/gcn3/insts/instructions.cc @@ -32123,9 +32123,9 @@ for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) { if (gpuDynInst->exec_mask[lane]) { - vdst0[lane] = (reinterpret_cast<VecElemU64*>( + vdst0[lane] = (reinterpret_cast<VecElemU32*>( gpuDynInst->d_data))[lane * 2]; - vdst1[lane] = (reinterpret_cast<VecElemU64*>( + vdst1[lane] = (reinterpret_cast<VecElemU32*>( gpuDynInst->d_data))[lane * 2 + 1]; } } diff --git a/src/arch/amdgpu/vega/insts/instructions.cc b/src/arch/amdgpu/vega/insts/instructions.cc index 45c8491..6c014bc 100644 --- a/src/arch/amdgpu/vega/insts/instructions.cc +++ b/src/arch/amdgpu/vega/insts/instructions.cc @@ -35665,13 +35665,13 @@ Inst_DS__DS_READ2ST64_B32::completeAcc(GPUDynInstPtr gpuDynInst) { VecOperandU32 vdst0(gpuDynInst, extData.VDST); - VecOperandU32 vdst1(gpuDynInst, extData.VDST + 2); + VecOperandU32 vdst1(gpuDynInst, extData.VDST + 1); for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) { if (gpuDynInst->exec_mask[lane]) { - vdst0[lane] = (reinterpret_cast<VecElemU64*>( + vdst0[lane] = (reinterpret_cast<VecElemU32*>( gpuDynInst->d_data))[lane * 2]; - vdst1[lane] = (reinterpret_cast<VecElemU64*>( + vdst1[lane] = (reinterpret_cast<VecElemU32*>( gpuDynInst->d_data))[lane * 2 + 1]; } } -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/70577?usp=email To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: public/gem5 Gerrit-Branch: develop Gerrit-Change-Id: Ifb110fc9a36ad198da7eaf86b1e3e37eccd3bb10 Gerrit-Change-Number: 70577 Gerrit-PatchSet: 2 Gerrit-Owner: Matthew Poremba <matthew.poremba@amd.com> Gerrit-Reviewer: Matt Sinclair <mattdsinclair@gmail.com> Gerrit-Reviewer: Matthew Poremba <matthew.poremba@amd.com> Gerrit-Reviewer: kokoro <noreply+kokoro@google.com>