From b9d35c7edc31f2f9a83bb090f288fdfd21e15457 Mon Sep 17 00:00:00 2001
From: Carlos Falquez <c.falquez@fz-juelich.de>
Date: Sat, 26 Jun 2021 14:42:12 +0200
Subject: [PATCH] base: Improved less-than operator for AddrRange

The current implementation of the less-than operator
for AddrRange compares intlvMatch values without first
checking that both ranges are interleaved.

This commit modifies the less-than operator to compare
intlvMatch values only if both regions are interleaved.
Otherwise, the operator returns whether the left
address range is interleaved.

This commit also adds AddrRangeMap unit tests
for interleaved address ranges.

JIRA: https://gem5.atlassian.net/browse/GEM5-1010

Change-Id: Id9f14d75d465d472c046995754bdccd441b9470c
Signed-off-by: Carlos Falquez <c.falquez@fz-juelich.de>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/47279
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Daniel Carvalho <odanrc@yahoo.com.br>
Tested-by: kokoro <noreply+kokoro@google.com>
---
 src/base/addr_range.hh          | 17 +++++--
 src/base/addr_range_map.test.cc | 88 +++++++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+), 5 deletions(-)

diff --git a/src/base/addr_range.hh b/src/base/addr_range.hh
index 12186fa9f2..405d8bafdd 100644
--- a/src/base/addr_range.hh
+++ b/src/base/addr_range.hh
@@ -593,12 +593,19 @@ class AddrRange
      */
     bool operator<(const AddrRange& r) const
     {
-        if (_start != r._start)
+        if (_start != r._start) {
             return _start < r._start;
-        else
-            // for now assume that the end is also the same, and that
-            // we are looking at the same interleaving bits
-            return intlvMatch < r.intlvMatch;
+        } else {
+            // For now assume that the end is also the same.
+            // If both regions are interleaved, assume same interleaving,
+            // and compare intlvMatch values.
+            // Otherwise, return true if this address range is interleaved.
+            if (interleaved() && r.interleaved()) {
+                return intlvMatch < r.intlvMatch;
+            } else {
+                return interleaved();
+            }
+        }
     }
 
     /**
diff --git a/src/base/addr_range_map.test.cc b/src/base/addr_range_map.test.cc
index fef3a894f6..74f6bffc50 100644
--- a/src/base/addr_range_map.test.cc
+++ b/src/base/addr_range_map.test.cc
@@ -40,6 +40,8 @@
 
 #include <gtest/gtest.h>
 
+#include <vector>
+
 #include "base/addr_range_map.hh"
 
 // Converted from legacy unit test framework
@@ -66,3 +68,89 @@ TEST(AddrRangeMapTest, LegacyTests)
 
     EXPECT_NE(r.contains(RangeIn(20, 30)), r.end());
 }
+
+/**
+ * Test AddrRangeMap with interleaved address ranges defined by bitmasks.
+ * An AddrRangeMap containing a set of N interleaved address ranges,
+ * defined with the same start and end address, and including all possible
+ * intlvMatch values 0..N-1, must contain the start address.
+ * For N-way interleaving, log2(N) selection masks are needed.
+ * For N = 16, define the masks as follows,
+ *
+ * masks[0] = 1 << 6
+ * masks[1] = 1 << 7
+ * masks[2] = 1 << 8
+ * masks[3] = 1 << 9
+ *
+ */
+TEST(AddrRangeMapTest, InterleavedTest1)
+{
+    const auto N = 16;
+    const auto masks = std::vector<Addr>{
+        0x40,
+        0x80,
+        0x100,
+        0x200
+    };
+    const Addr start = 0x80000000;
+    const Addr end   = 0xc0000000;
+
+    AddrRangeMap<int> r;
+    AddrRangeMap<int>::const_iterator i;
+
+    // populate AddrRangeMap with N-way interleaved address ranges
+    // for all intlvMatch values 0..N-1
+    for (int k=0; k < N; k++) {
+        r.insert(AddrRange(start, end, masks, k), k);
+    }
+    // find AddrRange element containing start address
+    i = r.contains(start);
+    // i must not be the past-the-end iterator
+    ASSERT_NE(i, r.end()) << "start address not found in AddrRangeMap";
+    // intlvMatch = 0 for start = 0x80000000
+    EXPECT_EQ(i->second, 0);
+}
+
+/**
+ * Test AddrRangeMap with interleaved address ranges defined by bitmasks.
+ * An AddrRangeMap containing a set of N interleaved address ranges,
+ * defined with the same start and end address, and including all possible
+ * intlvMatch values 0..N-1, must contain the start address.
+ * For N-way interleaving, log2(N) selection masks are needed.
+ * For N = 16, define the masks as described in the
+ * CMN-600 Technical Reference Manual [1], section 2.17.3
+ *
+ * masks[0] = 1 << 6 | 1 << 10 | 1 << 14 | .. | 1 << 50
+ * masks[1] = 1 << 7 | 1 << 11 | 1 << 15 | .. | 1 << 51
+ * masks[2] = 1 << 8 | 1 << 12 | 1 << 16 | .. | 1 << 48
+ * masks[3] = 1 << 9 | 1 << 13 | 1 << 17 | .. | 1 << 49
+ *
+ * [1] https://developer.arm.com/documentation/100180/0302
+ */
+TEST(AddrRangeMapTest, InterleavedTest2)
+{
+    const auto N = 16;
+    const auto masks = std::vector<Addr>{
+        0x4444444444440,
+        0x8888888888880,
+        0x1111111111100,
+        0x2222222222200
+    };
+    const Addr start = 0x80000000;
+    const Addr end   = 0xc0000000;
+
+    AddrRangeMap<int> r;
+    AddrRangeMap<int>::const_iterator i;
+
+    // populate AddrRangeMap with N-way interleaved address ranges
+    // for all intlvMatch values 0..N-1
+    for (int k=0; k < N; k++) {
+        r.insert(AddrRange(start, end, masks, k), k);
+    }
+    // find AddrRange element containing start address
+    i = r.contains(start);
+    // i must not be the past-the-end iterator
+    ASSERT_NE(i, r.end()) << "start address not found in AddrRangeMap";
+    // intlvMatch = 2 for start = 0x80000000
+    EXPECT_EQ(i->second, 2);
+}