From: Isaac Sánchez Barrera <isaac.sanchez@bsc.es>
Date: Fri, 6 Nov 2020 08:18:15 +0000 (+0100)
Subject: base: Fix `AddrRange::addIntlvBits(Addr)` and new test.
X-Git-Tag: develop-gem5-snapshot~490
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=55864843449b96af822c817214f91c992fbcc14d;p=gem5.git

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.

The test `AddrRangeTest.AddRemoveInterleavBitsAcrossRange` has been
updated have masks below bit 12, making the old code not pass the test.
A new `AddrRangeTest.AddRemoveInterleavBitsAcrossContiguousRange` test
has been added to include a case in which the previous code fails.  The
corrected code passes both tests.

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

Change-Id: I7d626a1f6ecf09a230fc18810d2dad2104d1a865
Signed-off-by: Isaac Sánchez Barrera <isaac.sanchez@bsc.es>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/37175
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Nikos Nikoleris <nikos.nikoleris@arm.com>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
---

diff --git a/src/base/addr_range.hh b/src/base/addr_range.hh
index e333b32c9..8a811d495 100644
--- a/src/base/addr_range.hh
+++ b/src/base/addr_range.hh
@@ -523,9 +523,10 @@ class AddrRange
             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 34921d83c..f2d4efafc 100644
--- a/src/base/addr_range.test.cc
+++ b/src/base/addr_range.test.cc
@@ -769,8 +769,8 @@ TEST(AddrRangeTest, AddRemoveInterleavBitsAcrossRange)
     std::vector<Addr> masks;
     masks.push_back(1 << 2);
     masks.push_back(1 << 3);
-    masks.push_back(1 << 16);
-    masks.push_back(1 << 30);
+    masks.push_back(1 << 7);
+    masks.push_back(1 << 11);
     uint8_t intlv_match = 0xF;
     AddrRange r(start, end, masks, intlv_match);
 
@@ -779,7 +779,35 @@ TEST(AddrRangeTest, AddRemoveInterleavBitsAcrossRange)
         /*
          * As intlv_match = 0xF, all the interleaved bits should be set.
          */
-        EXPECT_EQ(i | (1 << 2) | (1 << 3) | (1 << 16) | (1 << 30),
+        EXPECT_EQ(i | (1 << 2) | (1 << 3) | (1 << 7) | (1 << 11),
+                  r.addIntlvBits(removedBits));
+    }
+}
+
+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));
     }
 }