Matthew Poremba has uploaded this change for review. ( 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
---
M src/arch/amdgpu/vega/insts/instructions.cc
1 file changed, 45 insertions(+), 0 deletions(-)



diff --git a/src/arch/amdgpu/vega/insts/instructions.cc b/src/arch/amdgpu/vega/insts/instructions.cc
index f5b08b7..5612f29 100644
--- a/src/arch/amdgpu/vega/insts/instructions.cc
+++ b/src/arch/amdgpu/vega/insts/instructions.cc
@@ -1302,6 +1302,15 @@

         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.
+        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 +1382,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 +30561,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: 1
Gerrit-Owner: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

Reply via email to