From f99a3c1f96bb4a56cbb9b85d52829d606411649f Mon Sep 17 00:00:00 2001 From: Matthew Poremba Date: Fri, 16 Dec 2022 13:23:01 -0800 Subject: [PATCH] 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 Tested-by: kokoro Reviewed-by: Matt Sinclair --- src/arch/amdgpu/vega/insts/instructions.cc | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/arch/amdgpu/vega/insts/instructions.cc b/src/arch/amdgpu/vega/insts/instructions.cc index f5b08b7ce1..c9e57bc2f7 100644 --- a/src/arch/amdgpu/vega/insts/instructions.cc +++ b/src/arch/amdgpu/vega/insts/instructions.cc @@ -1302,6 +1302,21 @@ namespace VegaISA 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 @@ namespace VegaISA 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 @@ namespace VegaISA 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); + } } }