Matthew Poremba has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/55464 )

Change subject: arch-vega: Fix MUBUF out-of-bounds case 1
......................................................................

arch-vega: Fix MUBUF out-of-bounds case 1

Ported from https://gem5-review.googlesource.com/c/public/gem5/+/51127:

This patch upates the out-of-bounds check to properly check
against the correct buffer_offset, which is different depending
on if the const_swizzle_enable is true or false.

Change-Id: I9757226e62c587b679cab2a42f3616a5dca97e60
---
M src/arch/amdgpu/vega/insts/op_encodings.hh
1 file changed, 57 insertions(+), 39 deletions(-)



diff --git a/src/arch/amdgpu/vega/insts/op_encodings.hh b/src/arch/amdgpu/vega/insts/op_encodings.hh
index 109e680..a8040fa 100644
--- a/src/arch/amdgpu/vega/insts/op_encodings.hh
+++ b/src/arch/amdgpu/vega/insts/op_encodings.hh
@@ -594,6 +594,7 @@
             Addr stride = 0;
             Addr buf_idx = 0;
             Addr buf_off = 0;
+            Addr buffer_offset = 0;
             BufferRsrcDescriptor rsrc_desc;

             std::memcpy((void*)&rsrc_desc, s_rsrc_desc.rawDataPtr(),
@@ -616,42 +617,6 @@

                     buf_off = v_off[lane] + inst_offset;

-
-                    /**
-                     * Range check behavior causes out of range accesses to
- * to be treated differently. Out of range accesses return
-                     * 0 for loads and are ignored for stores. For
-                     * non-formatted accesses, this is done on a per-lane
-                     * basis.
-                     */
-                    if (stride == 0 || !rsrc_desc.swizzleEn) {
-                        if (buf_off + stride * buf_idx >=
-                            rsrc_desc.numRecords - s_offset.rawData()) {
- DPRINTF(VEGA, "mubuf out-of-bounds condition 1: "
-                                    "lane = %d, buffer_offset = %llx, "
-                                    "const_stride = %llx, "
-                                    "const_num_records = %llx\n",
-                                    lane, buf_off + stride * buf_idx,
-                                    stride, rsrc_desc.numRecords);
-                            oobMask.set(lane);
-                            continue;
-                        }
-                    }
-
-                    if (stride != 0 && rsrc_desc.swizzleEn) {
-                        if (buf_idx >= rsrc_desc.numRecords ||
-                            buf_off >= stride) {
- DPRINTF(VEGA, "mubuf out-of-bounds condition 2: "
-                                    "lane = %d, offset = %llx, "
-                                    "index = %llx, "
-                                    "const_num_records = %llx\n",
-                                    lane, buf_off, buf_idx,
-                                    rsrc_desc.numRecords);
-                            oobMask.set(lane);
-                            continue;
-                        }
-                    }
-
                     if (rsrc_desc.swizzleEn) {
                         Addr idx_stride = 8 << rsrc_desc.idxStride;
                         Addr elem_size = 2 << rsrc_desc.elemSize;
@@ -666,12 +631,50 @@
lane, idx_stride, elem_size, idx_msb, idx_lsb,
                                 off_msb, off_lsb);

-                        vaddr += ((idx_msb * stride + off_msb * elem_size)
-                            * idx_stride + idx_lsb * elem_size + off_lsb);
+ buffer_offset =(idx_msb * stride + off_msb * elem_size)
+                            * idx_stride + idx_lsb * elem_size + off_lsb;
                     } else {
-                        vaddr += buf_off + stride * buf_idx;
+                        buffer_offset = buf_off + stride * buf_idx;
                     }

+
+                    /**
+                     * Range check behavior causes out of range accesses to
+ * to be treated differently. Out of range accesses return
+                     * 0 for loads and are ignored for stores. For
+                     * non-formatted accesses, this is done on a per-lane
+                     * basis.
+                     */
+                    if (rsrc_desc.stride == 0 || !rsrc_desc.swizzleEn) {
+                        if (buffer_offset >=
+                            rsrc_desc.numRecords - s_offset.rawData()) {
+ DPRINTF(VEGA, "mubuf out-of-bounds condition 1: "
+                                    "lane = %d, buffer_offset = %llx, "
+                                    "const_stride = %llx, "
+                                    "const_num_records = %llx\n",
+                                    lane, buf_off + stride * buf_idx,
+                                    stride, rsrc_desc.numRecords);
+                            oobMask.set(lane);
+                            continue;
+                        }
+                    }
+
+                    if (rsrc_desc.stride != 0 && rsrc_desc.swizzleEn) {
+                        if (buf_idx >= rsrc_desc.numRecords ||
+                            buf_off >= stride) {
+ DPRINTF(VEGA, "mubuf out-of-bounds condition 2: "
+                                    "lane = %d, offset = %llx, "
+                                    "index = %llx, "
+                                    "const_num_records = %llx\n",
+                                    lane, buf_off, buf_idx,
+                                    rsrc_desc.numRecords);
+                            oobMask.set(lane);
+                            continue;
+                        }
+                    }
+
+                    vaddr += buffer_offset;
+
                     DPRINTF(VEGA, "Calculating mubuf address for lane %d: "
                             "vaddr = %llx, base_addr = %llx, "
"stride = %llx, buf_idx = %llx, buf_off = %llx\n",

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/55464
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: I9757226e62c587b679cab2a42f3616a5dca97e60
Gerrit-Change-Number: 55464
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
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to