Isaac Sánchez Barrera has submitted this change. (
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.
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.sanc...@bsc.es>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/37175
Tested-by: kokoro <noreply+kok...@google.com>
Reviewed-by: Nikos Nikoleris <nikos.nikole...@arm.com>
Reviewed-by: Bobby R. Bruce <bbr...@ucdavis.edu>
Maintainer: Nikos Nikoleris <nikos.nikole...@arm.com>
Maintainer: Bobby R. Bruce <bbr...@ucdavis.edu>
---
M src/base/addr_range.hh
M src/base/addr_range.test.cc
2 files changed, 35 insertions(+), 6 deletions(-)
Approvals:
Nikos Nikoleris: Looks good to me, approved; Looks good to me, approved
Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
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..f2d4efa 100644
--- a/src/base/addr_range.test.cc
+++ b/src/base/addr_range.test.cc
@@ -769,8 +769,8 @@
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 @@
/*
* 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));
}
}
--
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: 5
Gerrit-Owner: Isaac Sánchez Barrera <isaac.sanc...@bsc.es>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Isaac Sánchez Barrera <isaac.sanc...@bsc.es>
Gerrit-Reviewer: Nikos Nikoleris <nikos.nikole...@arm.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-CC: Adrià Armejach <adria.armej...@bsc.es>
Gerrit-CC: Andreas Sandberg <andreas.sandb...@arm.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
