Isaac Sánchez Barrera has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/37175 )

Change subject: base: Fix `AddrRange::addIntlvBits(Addr)` and new test.
......................................................................

base: Fix `AddrRange::addIntlvBits(Addr)` and new test.

The methods `AddrRange::removeIntlvBits(Addr)` and
`AddrRange::addIntlvBits(Addr)` should be the inverse of one another,
but the latter did not insert the blanks for filling the removed bits in
the correct positions.  Since the masks are ordered increasingly by the
position of the least significant bit of each mask, the lowest bit that
has to be inserted at each iteration is always `intlv_bit`, not needing
to be shifted to the left or right.  The bits that need to be copied
from the input address are `intlv_bit-1..0` at each iteration.

A new `AddrRangeTest.AddRemoveInterleavBitsAcrossContiguousRange` test
has been added to include a case in which the previous code fails but
the corrected code passes.

This function is not used anywhere other than the tests and the class
`ChannelAddr`.  However, it is needed to implement efficiently multibank
caches in the classic mode.

Change-Id: I7d626a1f6ecf09a230fc18810d2dad2104d1a865
Signed-off-by: Isaac Sánchez Barrera <isaac.sanc...@bsc.es>
---
M src/base/addr_range.hh
M src/base/addr_range.test.cc
2 files changed, 32 insertions(+), 3 deletions(-)



diff --git a/src/base/addr_range.hh b/src/base/addr_range.hh
index e333b32..8a811d4 100644
--- a/src/base/addr_range.hh
+++ b/src/base/addr_range.hh
@@ -523,9 +523,10 @@
             const int intlv_bit = masks_lsb[i];
             if (intlv_bit > 0) {
                 // on every iteration we add one bit from the input
-                // address, and therefore the lowest invtl_bit has
-                // also shifted to the left by i positions.
-                a = insertBits(a << 1, intlv_bit + i - 1, 0, a);
+                // address, but the lowest invtl_bit in the iteration is
+                // always in the right position because they are sorted
+                // increasingly from the LSB
+                a = insertBits(a << 1, intlv_bit - 1, 0, a);
             } else {
                 a <<= 1;
             }
diff --git a/src/base/addr_range.test.cc b/src/base/addr_range.test.cc
index 34921d8..13b32a5 100644
--- a/src/base/addr_range.test.cc
+++ b/src/base/addr_range.test.cc
@@ -784,6 +784,34 @@
     }
 }

+TEST(AddrRangeTest, AddRemoveInterleavBitsAcrossContiguousRange)
+{
+    /*
+     * This purpose of this test is to ensure that removing then adding
+     * interleaving bits has no net effect.
+     * E.g.:
+ * addr_range.addIntlvBits(add_range.removeIntlvBits(an_address)) should
+     * always return an_address.
+     */
+    Addr start = 0x00000;
+    Addr end   = 0x10000;
+    std::vector<Addr> masks;
+    masks.push_back(1 << 2);
+    masks.push_back(1 << 3);
+    masks.push_back(1 << 4);
+    uint8_t intlv_match = 0x7;
+    AddrRange r(start, end, masks, intlv_match);
+
+    for (Addr i = 0; i < 0xFFF; i++) {
+        Addr removedBits = r.removeIntlvBits(i);
+        /*
+         * As intlv_match = 0x7, all the interleaved bits should be set.
+         */
+        EXPECT_EQ(i | (1 << 2) | (1 << 3) | (1 << 4),
+                  r.addIntlvBits(removedBits));
+    }
+}
+
 TEST(AddrRangeTest, InterleavingAddressesGetOffset)
 {
     Addr start = 0x0002;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/37175
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: I7d626a1f6ecf09a230fc18810d2dad2104d1a865
Gerrit-Change-Number: 37175
Gerrit-PatchSet: 1
Gerrit-Owner: Isaac Sánchez Barrera <isaac.sanc...@bsc.es>
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