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 e333b32c9a3d3823858f4cae4220c1e15f904a8f..8a811d495ae62d583c7168c7d3ed2fa00ad83eab 100644 (file)
--- 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 34921d83cdb3eefe2d44bf3cec09e89559916a69..f2d4efafc0b2480d33d285436b71c5cce55765c1 100644 (file)
--- 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));
     }
 }