gem5-dev@gem5.org

The gem5 Developer List

View all threads

[S] Change in gem5/gem5[develop]: arch-vega: some Vega instructions don't use dest reg

MS
Matt Sinclair (Gerrit)
Wed, Jun 22, 2022 5:14 PM

Matt Sinclair has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/60649 )

Change subject: arch-vega: some Vega instructions don't use dest reg
......................................................................

arch-vega: some Vega instructions don't use dest reg

Some of the Vega scalar instructions (S_SETPC_B64, S_RFE_B64,
S_CBRANCH_JOIN, and S_SET_GPR_IDX_IDX) do not use the SDST scalar
destination register.  However, Vega's operand encoding function for the
SOP1 instruction type's class assumed all instructions used the
destination register, which results in an assert failure for these
instructions.

To resolve this, this commit updates the Vega SOP1 operand encoder to
ignore the destination register for these specific instructions.

Change-Id: I2f0d830f6264fc7f47c0694a2fd5da5d33d2ea0b
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/60649
Maintainer: Matt Sinclair mattdsinclair@gmail.com
Tested-by: kokoro noreply+kokoro@google.com
Maintainer: Matthew Poremba matthew.poremba@amd.com
Reviewed-by: Matthew Poremba matthew.poremba@amd.com

M src/arch/amdgpu/vega/insts/op_encodings.cc
1 file changed, 35 insertions(+), 2 deletions(-)

Approvals:
Matthew Poremba: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
Matt Sinclair: Looks good to me, approved

diff --git a/src/arch/amdgpu/vega/insts/op_encodings.cc
b/src/arch/amdgpu/vega/insts/op_encodings.cc
index 6f78b69..4abea2e 100644
--- a/src/arch/amdgpu/vega/insts/op_encodings.cc
+++ b/src/arch/amdgpu/vega/insts/op_encodings.cc
@@ -254,9 +254,18 @@
opNum++;
}

  •    reg = instData.SDST;
    
  •    dstOps.emplace_back(reg, getOperandSize(opNum), false,
    
  •    /*
    
  •      S_SETPC_B64, S_RFE_B64, S_CBRANCH_JOIN, and S_SET_GPR_IDX_IDX do  
    

not

  •      use SDST, so don't put anything on dstOps for them.
    
  •    */
    
  •    if ((instData.OP != 0x1D) /* S_SETPC_B64 (29 base 10) */ &&
    
  •        (instData.OP != 0x1F) /* S_RFE_B64 (31 base 10) */ &&
    
  •        (instData.OP != 0x2E) /* S_CBRANCH_JOIN (46 base 10) */ &&
    
  •        (instData.OP != 0x32)) /* S_SET_GPR_IDX_IDX (50 base 10) */ {
    
  •      reg = instData.SDST;
    
  •      dstOps.emplace_back(reg, getOperandSize(opNum), false,
                              isScalarReg(instData.SDST), false, false);
    
  •    }
    
        assert(srcOps.size() == numSrcRegOperands());
        assert(dstOps.size() == numDstRegOperands());
    

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/60649
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I2f0d830f6264fc7f47c0694a2fd5da5d33d2ea0b
Gerrit-Change-Number: 60649
Gerrit-PatchSet: 2
Gerrit-Owner: Matt Sinclair mattdsinclair@gmail.com
Gerrit-Reviewer: Alexandru Duțu alexandru.dutu@amd.com
Gerrit-Reviewer: Kyle Roarty kyleroarty1716@gmail.com
Gerrit-Reviewer: Matt Sinclair mattdsinclair@gmail.com
Gerrit-Reviewer: Matthew Poremba matthew.poremba@amd.com
Gerrit-Reviewer: kokoro noreply+kokoro@google.com
Gerrit-MessageType: merged

Matt Sinclair has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/60649 ) Change subject: arch-vega: some Vega instructions don't use dest reg ...................................................................... arch-vega: some Vega instructions don't use dest reg Some of the Vega scalar instructions (S_SETPC_B64, S_RFE_B64, S_CBRANCH_JOIN, and S_SET_GPR_IDX_IDX) do not use the SDST scalar destination register. However, Vega's operand encoding function for the SOP1 instruction type's class assumed all instructions used the destination register, which results in an assert failure for these instructions. To resolve this, this commit updates the Vega SOP1 operand encoder to ignore the destination register for these specific instructions. Change-Id: I2f0d830f6264fc7f47c0694a2fd5da5d33d2ea0b Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/60649 Maintainer: Matt Sinclair <mattdsinclair@gmail.com> Tested-by: kokoro <noreply+kokoro@google.com> Maintainer: Matthew Poremba <matthew.poremba@amd.com> Reviewed-by: Matthew Poremba <matthew.poremba@amd.com> --- M src/arch/amdgpu/vega/insts/op_encodings.cc 1 file changed, 35 insertions(+), 2 deletions(-) Approvals: Matthew Poremba: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass Matt Sinclair: Looks good to me, approved diff --git a/src/arch/amdgpu/vega/insts/op_encodings.cc b/src/arch/amdgpu/vega/insts/op_encodings.cc index 6f78b69..4abea2e 100644 --- a/src/arch/amdgpu/vega/insts/op_encodings.cc +++ b/src/arch/amdgpu/vega/insts/op_encodings.cc @@ -254,9 +254,18 @@ opNum++; } - reg = instData.SDST; - dstOps.emplace_back(reg, getOperandSize(opNum), false, + /* + S_SETPC_B64, S_RFE_B64, S_CBRANCH_JOIN, and S_SET_GPR_IDX_IDX do not + use SDST, so don't put anything on dstOps for them. + */ + if ((instData.OP != 0x1D) /* S_SETPC_B64 (29 base 10) */ && + (instData.OP != 0x1F) /* S_RFE_B64 (31 base 10) */ && + (instData.OP != 0x2E) /* S_CBRANCH_JOIN (46 base 10) */ && + (instData.OP != 0x32)) /* S_SET_GPR_IDX_IDX (50 base 10) */ { + reg = instData.SDST; + dstOps.emplace_back(reg, getOperandSize(opNum), false, isScalarReg(instData.SDST), false, false); + } assert(srcOps.size() == numSrcRegOperands()); assert(dstOps.size() == numDstRegOperands()); -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/60649 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: develop Gerrit-Change-Id: I2f0d830f6264fc7f47c0694a2fd5da5d33d2ea0b Gerrit-Change-Number: 60649 Gerrit-PatchSet: 2 Gerrit-Owner: Matt Sinclair <mattdsinclair@gmail.com> Gerrit-Reviewer: Alexandru Duțu <alexandru.dutu@amd.com> Gerrit-Reviewer: Kyle Roarty <kyleroarty1716@gmail.com> Gerrit-Reviewer: Matt Sinclair <mattdsinclair@gmail.com> Gerrit-Reviewer: Matthew Poremba <matthew.poremba@amd.com> Gerrit-Reviewer: kokoro <noreply+kokoro@google.com> Gerrit-MessageType: merged