misc: Use AddrRangeList more consistently in the AddrRange class.

We go through the trouble of defining an AddrRangeList typedef, but then
we don't use it consistently and use std::vector<AddrRange> instead.

This change converts the exclude method from using
std::vector<AddrRange> to AddrRangeList, and also adds a constructor
which takes an AddrRangeList.

Because there is a lot of code which uses the std::vector based
constructor, this change does not remove that method.

Change-Id: I1a03b25990025688aa760a67d3e7a2e8141384ce
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/50344
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Daniel Carvalho <odanrc@yahoo.com.br>
Tested-by: kokoro <noreply+kokoro@google.com>
This commit is contained in:
Gabe Black
2021-09-13 20:20:29 -07:00
parent ef2aaf995d
commit 416939c5c2
6 changed files with 90 additions and 135 deletions

View File

@@ -42,6 +42,7 @@
#define __BASE_ADDR_RANGE_HH__
#include <algorithm>
#include <iterator>
#include <list>
#include <vector>
@@ -53,6 +54,15 @@
namespace gem5
{
class AddrRange;
/**
* Convenience typedef for a collection of address ranges
*
* @ingroup api_addr_range
*/
typedef std::list<AddrRange> AddrRangeList;
/**
* The AddrRange class encapsulates an address range, and supports a
* number of tests to check if two ranges intersect, if a range
@@ -89,6 +99,48 @@ class AddrRange
/** The value to compare sel with. */
uint8_t intlvMatch;
protected:
struct Dummy {};
// The dummy parameter Dummy distinguishes this from the other two argument
// constructor which takes two Addrs.
template <class Iterator>
AddrRange(Dummy, Iterator begin_it, Iterator end_it)
: _start(1), _end(0), intlvMatch(0)
{
if (begin_it != end_it) {
// get the values from the first one and check the others
_start = begin_it->_start;
_end = begin_it->_end;
masks = begin_it->masks;
intlvMatch = begin_it->intlvMatch;
}
auto count = std::distance(begin_it, end_it);
// either merge if got all ranges or keep this equal to the single
// interleaved range
if (count > 1) {
fatal_if(count != (1ULL << masks.size()),
"Got %d ranges spanning %d interleaving bits.",
count, masks.size());
uint8_t match = 0;
for (auto it = begin_it; it != end_it; it++) {
fatal_if(!mergesWith(*it),
"Can only merge ranges with the same start, end "
"and interleaving bits, %s %s.", to_string(),
it->to_string());
fatal_if(it->intlvMatch != match,
"Expected interleave match %d but got %d when "
"merging.", match, it->intlvMatch);
++match;
}
masks.clear();
intlvMatch = 0;
}
}
public:
/**
@@ -215,40 +267,12 @@ class AddrRange
*
* @ingroup api_addr_range
*/
AddrRange(const std::vector<AddrRange>& ranges)
: _start(1), _end(0), intlvMatch(0)
{
if (!ranges.empty()) {
// get the values from the first one and check the others
_start = ranges.front()._start;
_end = ranges.front()._end;
masks = ranges.front().masks;
intlvMatch = ranges.front().intlvMatch;
}
// either merge if got all ranges or keep this equal to the single
// interleaved range
if (ranges.size() > 1) {
if (ranges.size() != (1ULL << masks.size()))
fatal("Got %d ranges spanning %d interleaving bits\n",
ranges.size(), masks.size());
uint8_t match = 0;
for (const auto& r : ranges) {
if (!mergesWith(r))
fatal("Can only merge ranges with the same start, end "
"and interleaving bits, %s %s\n", to_string(),
r.to_string());
if (r.intlvMatch != match)
fatal("Expected interleave match %d but got %d when "
"merging\n", match, r.intlvMatch);
++match;
}
masks.clear();
intlvMatch = 0;
}
}
AddrRange(std::vector<AddrRange> ranges)
: AddrRange(Dummy{}, ranges.begin(), ranges.end())
{}
AddrRange(std::list<AddrRange> ranges)
: AddrRange(Dummy{}, ranges.begin(), ranges.end())
{}
/**
* Determine if the range is interleaved or not.
@@ -611,15 +635,15 @@ class AddrRange
*
* @ingroup api_addr_range
*/
std::vector<AddrRange>
exclude(const std::vector<AddrRange> &exclude_ranges) const
AddrRangeList
exclude(const AddrRangeList &exclude_ranges) const
{
assert(!interleaved());
auto sorted_ranges = exclude_ranges;
std::sort(sorted_ranges.begin(), sorted_ranges.end());
sorted_ranges.sort();
std::vector<AddrRange> ranges;
std::list<AddrRange> ranges;
Addr next_start = start();
for (const auto &e : sorted_ranges) {
@@ -704,13 +728,6 @@ class AddrRange
}
};
/**
* Convenience typedef for a collection of address ranges
*
* @ingroup api_addr_range
*/
typedef std::list<AddrRange> AddrRangeList;
/**
* @ingroup api_addr_range
*/

View File

@@ -36,6 +36,7 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <cmath>
@@ -45,6 +46,8 @@
using namespace gem5;
using testing::ElementsAre;
TEST(AddrRangeTest, ValidRange)
{
AddrRange r;
@@ -921,10 +924,10 @@ TEST(AddrRangeTest, InterleavingNotEqualTo)
}
/*
* The AddrRange(std::vector<AddrRange>) constructor "merges" the interleaving
* The AddrRange(AddrRangeList) constructor "merges" the interleaving
* address ranges. It should be noted that this constructor simply checks that
* these interleaving addresses can be merged then creates a new address from
* the start and end addresses of the first address range in the vector.
* the start and end addresses of the first address range in the list.
*/
TEST(AddrRangeTest, MergingInterleavingAddressRanges)
{
@@ -942,7 +945,7 @@ TEST(AddrRangeTest, MergingInterleavingAddressRanges)
uint8_t intlv_match2 = 1;
AddrRange r2(start2, end2, masks2, intlv_match2);
std::vector<AddrRange> to_merge;
AddrRangeList to_merge;
to_merge.push_back(r1);
to_merge.push_back(r2);
@@ -956,7 +959,7 @@ TEST(AddrRangeTest, MergingInterleavingAddressRanges)
TEST(AddrRangeTest, MergingInterleavingAddressRangesOneRange)
{
/*
* In the case where there is just one range in the vector, the merged
* In the case where there is just one range in the list, the merged
* address range is equal to that range.
*/
Addr start = 0x0000;
@@ -966,7 +969,7 @@ TEST(AddrRangeTest, MergingInterleavingAddressRangesOneRange)
uint8_t intlv_match = 0;
AddrRange r(start, end, masks, intlv_match);
std::vector<AddrRange> to_merge;
AddrRangeList to_merge;
to_merge.push_back(r);
AddrRange output(to_merge);
@@ -1105,7 +1108,7 @@ TEST(AddrRangeTest, RangeSizeConstruction){
*/
TEST(AddrRangeTest, ExcludeAll)
{
const std::vector<AddrRange> exclude_ranges{
const AddrRangeList exclude_ranges{
AddrRange(0x0, 0x200)
};
@@ -1129,7 +1132,7 @@ TEST(AddrRangeTest, ExcludeAll)
*/
TEST(AddrRangeTest, ExcludeAllEqual)
{
const std::vector<AddrRange> exclude_ranges{
const AddrRangeList exclude_ranges{
AddrRange(0x0, 0x100)
};
@@ -1153,7 +1156,7 @@ TEST(AddrRangeTest, ExcludeAllEqual)
*/
TEST(AddrRangeTest, ExcludeAllMultiple)
{
const std::vector<AddrRange> exclude_ranges{
const AddrRangeList exclude_ranges{
AddrRange(0x0, 0x30),
AddrRange(0x30, 0x40),
AddrRange(0x40, 0x120)
@@ -1183,7 +1186,7 @@ TEST(AddrRangeTest, ExcludeAllMultiple)
*/
TEST(AddrRangeTest, ExcludeAllOverlapping)
{
const std::vector<AddrRange> exclude_ranges{
const AddrRangeList exclude_ranges{
AddrRange(0x0, 0x150),
AddrRange(0x140, 0x220)
};
@@ -1206,7 +1209,7 @@ TEST(AddrRangeTest, ExcludeAllOverlapping)
*/
TEST(AddrRangeTest, ExcludeEmpty)
{
const std::vector<AddrRange> exclude_ranges;
const AddrRangeList exclude_ranges;
AddrRange r(0x00, 0x100);
auto ranges = r.exclude(exclude_ranges);
@@ -1230,7 +1233,7 @@ TEST(AddrRangeTest, ExcludeEmpty)
*/
TEST(AddrRangeTest, NoExclusion)
{
const std::vector<AddrRange> exclude_ranges{
const AddrRangeList exclude_ranges{
AddrRange(0x100, 0x200)
};
@@ -1257,7 +1260,7 @@ TEST(AddrRangeTest, NoExclusion)
*/
TEST(AddrRangeTest, DoubleExclusion)
{
const std::vector<AddrRange> exclude_ranges{
const AddrRangeList exclude_ranges{
AddrRange(0x000, 0x130),
AddrRange(0x140, 0x170),
};
@@ -1269,8 +1272,7 @@ TEST(AddrRangeTest, DoubleExclusion)
auto ranges = r.exclude(exclude_ranges);
EXPECT_EQ(ranges.size(), 2);
EXPECT_EQ(ranges[0], expected_range1);
EXPECT_EQ(ranges[1], expected_range2);
EXPECT_THAT(ranges, ElementsAre(expected_range1, expected_range2));
}
/*
@@ -1289,7 +1291,7 @@ TEST(AddrRangeTest, DoubleExclusion)
*/
TEST(AddrRangeTest, MultipleExclusion)
{
const std::vector<AddrRange> exclude_ranges{
const AddrRangeList exclude_ranges{
AddrRange(0x000, 0x130),
AddrRange(0x140, 0x170),
AddrRange(0x180, 0x210)
@@ -1302,8 +1304,7 @@ TEST(AddrRangeTest, MultipleExclusion)
auto ranges = r.exclude(exclude_ranges);
EXPECT_EQ(ranges.size(), 2);
EXPECT_EQ(ranges[0], expected_range1);
EXPECT_EQ(ranges[1], expected_range2);
EXPECT_THAT(ranges, ElementsAre(expected_range1, expected_range2));
}
/*
@@ -1324,7 +1325,7 @@ TEST(AddrRangeTest, MultipleExclusion)
*/
TEST(AddrRangeTest, MultipleExclusionOverlapping)
{
const std::vector<AddrRange> exclude_ranges{
const AddrRangeList exclude_ranges{
AddrRange(0x000, 0x130),
AddrRange(0x140, 0x170),
AddrRange(0x150, 0x210)
@@ -1336,7 +1337,7 @@ TEST(AddrRangeTest, MultipleExclusionOverlapping)
auto ranges = r.exclude(exclude_ranges);
EXPECT_EQ(ranges.size(), 1);
EXPECT_EQ(ranges[0], expected_range1);
EXPECT_THAT(ranges, ElementsAre(expected_range1));
}
/*
@@ -1359,7 +1360,7 @@ TEST(AddrRangeTest, MultipleExclusionOverlapping)
*/
TEST(AddrRangeTest, ExclusionOverlapping)
{
const std::vector<AddrRange> exclude_ranges{
const AddrRangeList exclude_ranges{
AddrRange(0x120, 0x180),
AddrRange(0x130, 0x170)
};
@@ -1371,8 +1372,7 @@ TEST(AddrRangeTest, ExclusionOverlapping)
auto ranges = r.exclude(exclude_ranges);
EXPECT_EQ(ranges.size(), 2);
EXPECT_EQ(ranges[0], expected_range1);
EXPECT_EQ(ranges[1], expected_range2);
EXPECT_THAT(ranges, ElementsAre(expected_range1, expected_range2));
}
/*
@@ -1393,7 +1393,7 @@ TEST(AddrRangeTest, ExclusionOverlapping)
*/
TEST(AddrRangeTest, MultipleExclusionUnsorted)
{
const std::vector<AddrRange> exclude_ranges{
const AddrRangeList exclude_ranges{
AddrRange(0x180, 0x210),
AddrRange(0x000, 0x130),
AddrRange(0x140, 0x170)
@@ -1406,8 +1406,7 @@ TEST(AddrRangeTest, MultipleExclusionUnsorted)
auto ranges = r.exclude(exclude_ranges);
EXPECT_EQ(ranges.size(), 2);
EXPECT_EQ(ranges[0], expected_range1);
EXPECT_EQ(ranges[1], expected_range2);
EXPECT_THAT(ranges, ElementsAre(expected_range1, expected_range2));
}
/*
@@ -1426,7 +1425,7 @@ TEST(AddrRangeDeathTest, ExcludeInterleavingRanges)
#ifdef NDEBUG
GTEST_SKIP() << "Skipping as assetions are stripped from fast builds.";
#endif
const std::vector<AddrRange> exclude_ranges{
const AddrRangeList exclude_ranges{
AddrRange(0x180, 0x210),
};

View File

@@ -728,7 +728,6 @@ class MetaSimObject(type):
#include "base/compiler.hh"
#include "params/$cls.hh"
#include "python/pybind11/core.hh"
#include "sim/init.hh"
#include "sim/sim_object.hh"

View File

@@ -865,15 +865,10 @@ class AddrRange(ParamValue):
self.masks, int(self.intlvMatch))
def exclude(self, ranges):
from _m5.range import AddrRangeVector
# The wrapped C++ class is assuming an AddrRangeVector
# We are therefore converting to it before excluding ranges
# and reconverting it into a list of AddrRange before returning
pybind_exclude = AddrRangeVector([ r.getValue() for r in ranges ])
pybind_exclude = list([ r.getValue() for r in ranges ])
pybind_include = self.getValue().exclude(pybind_exclude)
return [ AddrRange(r.start(), r.end()) for r in pybind_include ]
return list([ AddrRange(r.start(), r.end()) for r in pybind_include ])
# Boolean parameter type. Python doesn't let you subclass bool, since
# it doesn't want to let you create multiple instances of True and

View File

@@ -39,11 +39,10 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include "pybind11/operators.h"
#include "pybind11/pybind11.h"
#include "pybind11/stl.h"
#include "python/pybind11/core.hh"
#include <ctime>
#include "base/addr_range.hh"
@@ -158,10 +157,6 @@ init_range(py::module_ &m_native)
.def("exclude", &AddrRange::exclude)
;
// We need to make vectors of AddrRange opaque to avoid weird
// memory allocation issues in PyBind's STL wrappers.
py::bind_vector<std::vector<AddrRange>>(m, "AddrRangeVector");
m.def("RangeEx", &RangeEx);
m.def("RangeIn", &RangeIn);
m.def("RangeSize", &RangeSize);

View File

@@ -1,50 +0,0 @@
/*
* Copyright (c) 2017 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
* not be construed as granting a license to any other intellectual
* property including but not limited to intellectual property relating
* to a hardware implementation of the functionality of the software
* licensed hereunder. You may use the software subject to the license
* terms below provided that you ensure that this notice is replicated
* unmodified and in its entirety in all distributions of the software,
* modified or unmodified, in source code or in binary form.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
* met: redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer;
* redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution;
* neither the name of the copyright holders nor the names of its
* contributors may be used to endorse or promote products derived from
* this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#ifndef __PYTHON_PYBIND11_CORE_HH__
#define __PYTHON_PYBIND11_CORE_HH__
#include "pybind11/cast.h"
#include "pybind11/stl_bind.h"
#include <vector>
#include "base/addr_range.hh"
PYBIND11_MAKE_OPAQUE(std::vector<gem5::AddrRange>);
#endif // __PYTHON_PYBIND11_CORE_HH__