Matthew Poremba has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/66751?usp=email )
Change subject: arch-vega: Fix signed BFE instructions
......................................................................
arch-vega: Fix signed BFE instructions
The bitfield extract instructions come in unsigned and signed variants.
The documentation on this is not correct, however the GCN3 documentation
gives some clues. The instruction should extract an N-bit integer where
N is defined in a source operand starting at some bit also defined by a
source operand. For signed variants of this instruction, the N-bit
integer should be sign extended but is currently not.
This changeset does sign extension using the runtime value of N by ORing
the upper bits with ones if the most significant bit is one. This was
verified by writing these instructions in assembly and running on a real
GPU. Changes are made to v_bfe_i32, s_bfe_i32, and s_bfe_i64.
Change-Id: Ia192f5940200c6de48867b02f709a7f1b2daa974
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/66751
Maintainer: Matt Sinclair <mattdsincl...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
Reviewed-by: Matt Sinclair <mattdsincl...@gmail.com>
---
M src/arch/amdgpu/vega/insts/instructions.cc
1 file changed, 55 insertions(+), 0 deletions(-)
Approvals:
Matt Sinclair: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/amdgpu/vega/insts/instructions.cc
b/src/arch/amdgpu/vega/insts/instructions.cc
index f5b08b7..c9e57bc 100644
--- a/src/arch/amdgpu/vega/insts/instructions.cc
+++ b/src/arch/amdgpu/vega/insts/instructions.cc
@@ -1302,6 +1302,21 @@
sdst = (src0.rawData() >> bits(src1.rawData(), 4, 0))
& ((1 << bits(src1.rawData(), 22, 16)) - 1);
+
+ // Above extracted a signed int of size src1[22:16] bits which
needs
+ // to be signed-extended. Check if the MSB of our src1[22:16]-bit
+ // integer is 1, and sign extend it is.
+ //
+ // Note: The description in the Vega ISA manual does not mention to
+ // sign-extend the result. An update description can be found in
the
+ // more recent RDNA3 manual here:
+ // https://developer.amd.com/wp-content/resources/
+ // RDNA3_Shader_ISA_December2022.pdf
+ if (sdst.rawData() >> (bits(src1.rawData(), 22, 16) - 1)) {
+ sdst = sdst.rawData()
+ | (0xffffffff << bits(src1.rawData(), 22, 16));
+ }
+
scc = sdst.rawData() ? 1 : 0;
sdst.write();
@@ -1373,6 +1388,14 @@
sdst = (src0.rawData() >> bits(src1.rawData(), 5, 0))
& ((1 << bits(src1.rawData(), 22, 16)) - 1);
+
+ // Above extracted a signed int of size src1[22:16] bits which
needs
+ // to be signed-extended. Check if the MSB of our src1[22:16]-bit
+ // integer is 1, and sign extend it is.
+ if (sdst.rawData() >> (bits(src1.rawData(), 22, 16) - 1)) {
+ sdst = sdst.rawData()
+ | 0xffffffffffffffff << bits(src1.rawData(), 22, 16);
+ }
scc = sdst.rawData() ? 1 : 0;
sdst.write();
@@ -30544,6 +30567,13 @@
if (wf->execMask(lane)) {
vdst[lane] = (src0[lane] >> bits(src1[lane], 4, 0))
& ((1 << bits(src2[lane], 4, 0)) - 1);
+
+ // Above extracted a signed int of size src2 bits which
needs
+ // to be signed-extended. Check if the MSB of our src2-bit
+ // integer is 1, and sign extend it is.
+ if (vdst[lane] >> (bits(src2[lane], 4, 0) - 1)) {
+ vdst[lane] |= 0xffffffff << bits(src2[lane], 4, 0);
+ }
}
}
--
To view, visit
https://gem5-review.googlesource.com/c/public/gem5/+/66751?usp=email
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: Ia192f5940200c6de48867b02f709a7f1b2daa974
Gerrit-Change-Number: 66751
Gerrit-PatchSet: 4
Gerrit-Owner: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-Reviewer: Matt Sinclair <mattdsincl...@gmail.com>
Gerrit-Reviewer: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
