Matthew Poremba has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/55464 )
(
2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
)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 updates 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
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/55464
Reviewed-by: Matt Sinclair <mattdsincl...@gmail.com>
Maintainer: Matt Sinclair <mattdsincl...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/arch/amdgpu/vega/insts/op_encodings.hh
1 file changed, 61 insertions(+), 39 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/op_encodings.hh
b/src/arch/amdgpu/vega/insts/op_encodings.hh
index bb45b74..da16dbd 100644
--- a/src/arch/amdgpu/vega/insts/op_encodings.hh
+++ b/src/arch/amdgpu/vega/insts/op_encodings.hh
@@ -632,6 +632,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(),
@@ -654,42 +655,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;
@@ -704,12 +669,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: 4
Gerrit-Owner: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-Reviewer: Kyle Roarty <kyleroarty1...@gmail.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
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s