Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/38478 )
Change subject: base,cpu: Simplify the CircularQueue class significantly.
......................................................................
base,cpu: Simplify the CircularQueue class significantly.
This class had been trying to keep all indices within the modulus of the
queue size, and to use all elements in the underlying storage by making
the empty and full conditions alias, differentiated by a bool. To keep
track of the difference between a storage location on one trip around
the queue vs other times around, ie an alias once the indices had
wrapped, it also keep track of a "round" value in both the queue itself,
and any iterators it created.
All this bookkeeping significantly complicated the data structure.
Instead, this change modifies it to keep track of a monotonically
increasing index which is wrapped at the time it's used. Only the head
index and current size need to be tracked in the queue itself, and only
a pointer to the queue and an index need to be tracked in the iterators.
Theoretically, it's possible that this value could overflow eventually
since it increases forever, unlike before where the index wrapped and
was never larger than the queue's capacity. In practice, the type of the
index was changed from a uint32_t to a size_t, probably a 64 bit value
in modern systems, which will hold much larger values. Also, the round
counter and the index values together acted like a smaller than 64 bit
value anyway, since the round counter would overflow after 2^32 times
around a less than 2^32 entry queue.
One minor interface difference is that the head() and tail() values
returned by the queue are no longer pre-wrapped to be modulo the queue's
capacity. As long as consumers don't try to be overly clever and feed in
fixed values, do their own bounds checking, etc., something that would
be cumbersome considering the wrapping nature of the structure, this
shouldn't be an issue.
Also, since external consumers no longer need to worry about wrapping,
since only one of them was used in only one place, and because they
weren't even marked as part of the interface, the modulo helper
functions have been eliminated from the queue. If other code wants to
perform modulo arithmetic for some reason (which the queue no longer
requires) they can accomplish basically the same thing in basically the
same amount of code using normal math.
Also, rather than inherit from std::vector, this change makes the vector
internal to the queue. That prevents methods of the vector that aren't
aware of the circular nature of the structure from leaking out if
they're not overridden or otherwise proactively blocked.
On top of simplifying the implementation, this also makes it perform
*slightly* better. To measure that, I ran the following command:
$ time build/ARM/base/circular_queue.test.opt --gtest_repeat=100000 >
/dev/null
and found a few percent improvement in total run time. While this
difference was small and not measured against realistic usage of the
data structure, it was still measurable, and at minimum doesn't seem to
have hurt performance.
Change-Id: Ic2baa28de135be7086fa94579bbec451d69b3b15
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38478
Reviewed-by: Daniel Carvalho <oda...@yahoo.com.br>
Maintainer: Gabe Black <gabe.bl...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/base/circular_queue.hh
M src/base/circular_queue.test.cc
M src/cpu/base_dyn_inst.hh
M src/cpu/o3/lsq_unit_impl.hh
4 files changed, 99 insertions(+), 359 deletions(-)
Approvals:
Daniel Carvalho: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/base/circular_queue.hh b/src/base/circular_queue.hh
index 69e59ab..5425633 100644
--- a/src/base/circular_queue.hh
+++ b/src/base/circular_queue.hh
@@ -46,126 +46,49 @@
#include <vector>
/** Circular queue.
- * Circular queue implemented on top of a standard vector. Instead of using
- * a sentinel entry, we use a boolean to distinguish the case in which the
- * queue is full or empty.
- * Thus, a circular queue is represented by the 5-tuple
- * (Capacity, IsEmpty?, Head, Tail, Round)
- * Where:
- * - Capacity is the size of the underlying vector.
- * - IsEmpty? can be T or F.
- * - Head is the index in the vector of the first element of the queue.
- * - Tail is the index in the vector of the last element of the queue.
- * - Round is the counter of how many times the Tail has wrapped around.
- * A queue is empty when
- * Head == (Tail + 1 mod Capacity) && IsEmpty?.
- * Conversely, a queue is full when
- * Head == (Tail + 1 mod Capacity) && !IsEmpty?.
- * Comments may show depictions of the underlying vector in the following
- * format: '|' delimit the 'cells' of the underlying vector. '-' represents
- * an element of the vector that is out-of-bounds of the circular queue,
- * while 'o' represents and element that is inside the bounds. The
- * characters '[' and ']' are added to mark the entries that hold the head
- * and tail of the circular queue respectively.
- * E.g.:
- * - Empty queues of capacity 4:
- * (4,T,1,0,_): |-]|[-|-|-| (4,T,3,2): |-|-|-]|[-|
- * - Full queues of capacity 4:
- * (4,F,1,0,_): |o]|[o|o|o| (4,F,3,2): |o|o|o]|[o|
- * - Queues of capacity 4 with 2 elements:
- * (4,F,0,1,_): |[o|o]|-|-| (4,F,3,0): |o]|-|-|[o|
+ * Circular queue implemented in a standard vector. All indices are
+ * monotonically increasing, and modulo is used at access time to alias
them
+ * down to the actual storage.
*
- * The Round number is only relevant for checking validity of indices,
- * therefore it will be omitted or shown as '_'
+ * The queue keeps track of two pieces of state, a head index, which is the
+ * index of the next element to come out of the queue, and a size which is
how
+ * many valid elements are currently in the queue. Size can increase to,
but
+ * never exceed, the capacity of the queue.
+ *
+ * In theory the index may overflow at some point, but since it's a 64 bit
+ * value that would take a very long time.
*
* @tparam T Type of the elements in the queue
*
* @ingroup api_base_utils
*/
template <typename T>
-class CircularQueue : private std::vector<T>
+class CircularQueue
{
protected:
- using Base = std::vector<T>;
- using typename Base::reference;
- using typename Base::const_reference;
- const uint32_t _capacity;
- uint32_t _head = 1;
- uint32_t _tail = 0;
- uint32_t _empty = true;
+ std::vector<T> data;
- /** Counter for how many times the tail wraps around.
- * Some parts of the code rely on getting the past the end iterator,
and
- * expect to use it after inserting on the tail. To support this
without
- * ambiguity, we need the round number to guarantee that it did not
become
- * a before-the-beginning iterator.
- */
- uint32_t _round = 0;
-
- /** General modular addition. */
- static uint32_t
- moduloAdd(uint32_t op1, uint32_t op2, uint32_t size)
- {
- return (op1 + op2) % size;
- }
-
- /** General modular subtraction. */
- static uint32_t
- moduloSub(uint32_t op1, uint32_t op2, uint32_t size)
- {
- int32_t ret = sub(op1, op2, size);
- return ret >= 0 ? ret : ret + size;
- }
-
- static int32_t
- sub(uint32_t op1, uint32_t op2, uint32_t size)
- {
- if (op1 > op2)
- return (op1 - op2) % size;
- else
- return -((op2 - op1) % size);
- }
-
- void
- increase(uint32_t& v, size_t delta=1)
- {
- v = moduloAdd(v, delta, _capacity);
- }
-
- void
- decrease(uint32_t& v)
- {
- v = (v ? v : _capacity) - 1;
- }
+ using reference = typename std::vector<T>::reference;
+ using const_reference = typename std::vector<T>::const_reference;
+ const size_t _capacity;
+ size_t _size = 0;
+ size_t _head = 1;
/** Iterator to the circular queue.
- * iterator implementation to provide the circular-ness that the
- * standard std::vector<T>::iterator does not implement.
- * Iterators to a queue are represented by a pair of a character and
the
- * round counter. For the character, '*' denotes the element pointed
to by
- * the iterator if it is valid. 'x' denotes the element pointed to by
the
- * iterator when it is BTB or PTE.
- * E.g.:
- * - Iterator to the head of a queue of capacity 4 with 2 elems.
- * (4,F,0,1,R): |[(*,R)|o]|-|-| (4,F,3,0): |o]|-|-|[(*,R)|
- * - Iterator to the tail of a queue of capacity 4 with 2 elems.
- * (4,F,0,1,R): |[o|(*,R)]|-|-| (4,F,3,0): |(*,R)]|-|-|[o|
- * - Iterator to the end of a queue of capacity 4 with 2 elems.
- * (4,F,0,1,R): |[o|o]|(x,R)|-| (4,F,3,0): |o]|(x,R)|-|[o|
+ * iterator implementation to provide wrap around indexing which the
+ * vector iterator does not.
*/
public:
struct iterator
{
public:
CircularQueue* _cq = nullptr;
- uint32_t _idx = 0;
- uint32_t _round = 0;
+ size_t _idx = 0;
/**
* @ingroup api_base_utils
*/
- iterator(CircularQueue* cq, uint32_t idx, uint32_t round)
- : _cq(cq), _idx(idx), _round(round) {}
+ iterator(CircularQueue* cq, size_t idx) : _cq(cq), _idx(idx) {}
iterator() = default;
/**
@@ -192,9 +115,7 @@
/**
* @ingroup api_base_utils
*/
- iterator(const iterator& it)
- : _cq(it._cq), _idx(it._idx), _round(it._round)
- {}
+ iterator(const iterator& it) : _cq(it._cq), _idx(it._idx) {}
/**
* @ingroup api_base_utils
@@ -204,30 +125,21 @@
{
_cq = it._cq;
_idx = it._idx;
- _round = it._round;
return *this;
}
/**
* Test dereferenceability.
* An iterator is dereferenceable if it is pointing to a non-null
- * circular queue, it is not the past-the-end iterator and the
- * index is a valid index to that queue. PTE test is required to
- * distinguish between:
- * - An iterator to the first element of a full queue
- * (4,F,1,0): |o]|[*|o|o|
- * - The end() iterator of a full queue
- * (4,F,1,0): |o]|x[o|o|o|
- * Sometimes, though, users will get the PTE iterator and expect it
- * to work after growing the buffer on the tail, so we have to
- * check if the iterator is still PTE.
+ * circular queue, and the index is within the current range of the
+ * queue.
*
* @ingroup api_base_utils
*/
bool
dereferenceable() const
{
- return _cq != nullptr && _cq->isValidIdx(_idx, _round);
+ return _cq != nullptr && _cq->isValidIdx(_idx);
}
/** InputIterator. */
@@ -235,17 +147,14 @@
/**
* Equality operator.
* Two iterators must point to the same, possibly null, circular
- * queue and the same element on it, including PTE, to be equal.
- * In case the clients the the PTE iterator and then grow on the
back
- * and expect it to work, we have to check if the PTE is still PTE
+ * queue and the same element on it to be equal.
*
* @ingroup api_base_utils
*/
bool
operator==(const iterator& that) const
{
- return _cq == that._cq && _idx == that._idx &&
- _round == that._round;
+ return _cq == that._cq && _idx == that._idx;
}
/**
@@ -285,12 +194,12 @@
*
* @ingroup api_base_utils
*/
- pointer operator->() { return &((*_cq)[_idx]); }
+ pointer operator->() { return &**this; }
/**
* @ingroup api_base_utils
*/
- const_pointer operator->() const { return &((*_cq)[_idx]); }
+ const_pointer operator->() const { return &**this; }
/**
* Pre-increment operator.
@@ -300,10 +209,7 @@
iterator&
operator++()
{
- /* this has to be dereferenceable. */
- _cq->increase(_idx);
- if (_idx == 0)
- ++_round;
+ ++_idx;
return *this;
}
@@ -325,23 +231,6 @@
*/
/** BidirectionalIterator requirements. */
- private:
- /** Test decrementability.
- * An iterator to a non-null circular queue is not-decrementable
- * if it is pointing to the head element, unless the queue is full
- * and we are talking about the past-the-end iterator. In that
case,
- * the iterator round equals the cq round unless the head is at the
- * zero position and the round is one more than the cq round.
- */
- bool
- decrementable() const
- {
- return _cq && !(_idx == _cq->head() &&
- (_cq->empty() ||
- (_idx == 0 && _round != _cq->_round + 1) ||
- (_idx !=0 && _round != _cq->_round)));
- }
-
public:
/**
* Pre-decrement operator.
@@ -352,10 +241,8 @@
operator--()
{
/* this has to be decrementable. */
- assert(decrementable());
- if (_idx == 0)
- --_round;
- _cq->decrease(_idx);
+ assert(_cq && _idx > _cq->head());
+ --_idx;
return *this;
}
@@ -380,9 +267,7 @@
iterator&
operator+=(const difference_type& t)
{
- assert(_cq);
- _round += (t + _idx) / _cq->capacity();
- _idx = _cq->moduloAdd(_idx, t);
+ _idx += t;
return *this;
}
@@ -392,15 +277,8 @@
iterator&
operator-=(const difference_type& t)
{
- assert(_cq);
-
- /* C does not do euclidean division, so we have to adjust */
- if (t >= 0) {
- _round += (-t + _idx) / _cq->capacity();
- _idx = _cq->moduloSub(_idx, t);
- } else {
- *this += -t;
- }
+ assert(_cq && _idx >= _cq->head() + t);
+ _idx -= t;
return *this;
}
@@ -457,13 +335,7 @@
difference_type
operator-(const iterator& that)
{
- /* If a is already at the end, we can safely return 0. */
- auto ret = _cq->sub(this->_idx, that._idx, _cq->capacity());
-
- if (this->_round != that._round) {
- ret += ((this->_round - that._round) * _cq->capacity());
- }
- return ret;
+ return (ssize_t)_idx - (ssize_t)that._idx;
}
/**
@@ -487,9 +359,7 @@
bool
operator<(const iterator& that) const
{
- assert(_cq && that._cq == _cq);
- return (this->_round < that._round) ||
- (this->_round == that._round && _idx < that._idx);
+ return _idx < that._idx;
}
/**
@@ -517,15 +387,26 @@
/**
* @ingroup api_base_utils
*/
- using Base::operator[];
+ template <typename Idx>
+ typename std::enable_if_t<std::is_integral<Idx>::value, reference>
+ operator[](const Idx& index)
+ {
+ assert(index >= 0);
+ return data[index % _capacity];
+ }
+
+ template <typename Idx>
+ typename std::enable_if_t<std::is_integral<Idx>::value,
const_reference>
+ operator[](const Idx& index) const
+ {
+ assert(index >= 0);
+ return data[index % _capacity];
+ }
/**
* @ingroup api_base_utils
*/
- explicit CircularQueue(uint32_t size=0) : _capacity(size)
- {
- Base::resize(size);
- }
+ explicit CircularQueue(size_t size=0) : data(size), _capacity(size) {}
/**
* Remove all the elements in the queue.
@@ -539,9 +420,7 @@
flush()
{
_head = 1;
- _round = 0;
- _tail = 0;
- _empty = true;
+ _size = 0;
}
/**
@@ -550,82 +429,28 @@
bool
isValidIdx(size_t idx) const
{
- /* An index is invalid if:
- * - The queue is empty.
- * (6,T,3,2): |-|-|-]|[-|-|x|
- * - head is small than tail and:
- * - It is greater than both head and tail.
- * (6,F,1,3): |-|[o|o|o]|-|x|
- * - It is less than both head and tail.
- * (6,F,1,3): |x|[o|o|o]|-|-|
- * - It is greater than the tail and not than the head.
- * (6,F,4,1): |o|o]|-|x|[o|o|
- */
- return !(_empty || (
- (_head < _tail) && (
- (_head < idx && _tail < idx) ||
- (_head > idx && _tail > idx)
- )) || (_tail < idx && idx < _head));
- }
-
- /**
- * Test if the index is in the range of valid elements.
- * The round counter is used to disambiguate aliasing.
- */
- bool
- isValidIdx(size_t idx, uint32_t round) const
- {
- /* An index is valid if:
- * - The queue is not empty.
- * - round == R and
- * - index <= tail (if index > tail, that would be PTE)
- * - Either:
- * - head <= index
- * (6,F,1,3,R): |-|[o|(*,r)|o]|-|-|
- * - head > tail
- * (6,F,5,3,R): |o|o|(*,r)|o]|-|[o|
- * The remaining case means the the iterator is BTB:
- * (6,F,3,4,R): |-|-|(x,r)|[o|o]|-|
- * - round + 1 == R and:
- * - index > tail. If index <= tail, that would be BTB:
- * (6,F,2,3,r): | -|- |[(*,r)|o]|-|-|
- * (6,F,0,1,r+1): |[o|o]| (x,r)|- |-|-|
- * (6,F,0,3,r+1): |[o|o | (*,r)|o]|-|-|
- * - index >= head. If index < head, that would be BTB:
- * (6,F,5,2,R): |o|o]|-|-|(x,r)|[o|
- * - head > tail. If head <= tail, that would be BTB:
- * (6,F,3,4,R): |[o|o]|(x,r)|-|-|-|
- * Other values of the round meand that the index is PTE or
BTB
- */
- return (!_empty && (
- (round == _round && idx <= _tail && (
- _head <= idx || _head > _tail)) ||
- (round + 1 == _round &&
- idx > _tail &&
- idx >= _head &&
- _head > _tail)
- ));
+ return _head <= idx && idx < (_head + _size);
}
/**
* @ingroup api_base_utils
*/
- reference front() { return (*this)[_head]; }
+ reference front() { return (*this)[head()]; }
/**
* @ingroup api_base_utils
*/
- reference back() { return (*this)[_tail]; }
+ reference back() { return (*this)[tail()]; }
/**
* @ingroup api_base_utils
*/
- uint32_t head() const { return _head; }
+ size_t head() const { return _head; }
/**
* @ingroup api_base_utils
*/
- uint32_t tail() const { return _tail; }
+ size_t tail() const { return _head + _size - 1; }
/**
* @ingroup api_base_utils
@@ -635,34 +460,11 @@
/**
* @ingroup api_base_utils
*/
- uint32_t
- size() const
- {
- if (_empty)
- return 0;
- else if (_head <= _tail)
- return _tail - _head + 1;
- else
- return _capacity - _head + _tail + 1;
- }
-
- uint32_t
- moduloAdd(uint32_t s1, uint32_t s2) const
- {
- return moduloAdd(s1, s2, _capacity);
- }
-
- uint32_t
- moduloSub(uint32_t s1, uint32_t s2) const
- {
- return moduloSub(s1, s2, _capacity);
- }
+ size_t size() const { return _size; }
/** Circularly increase the head pointer.
* By increasing the head pointer we are removing elements from
* the begin of the circular queue.
- * Check that the queue is not empty. And set it to empty if it
- * had only one value prior to insertion.
*
* @params num_elem number of elements to remove
*
@@ -671,13 +473,9 @@
void
pop_front(size_t num_elem=1)
{
- if (num_elem == 0)
- return;
- auto hIt = begin();
- hIt += num_elem;
- assert(hIt <= end());
- _empty = hIt == end();
- _head = hIt._idx;
+ assert(num_elem <= size());
+ _head += num_elem;
+ _size -= num_elem;
}
/**
@@ -688,11 +486,8 @@
void
pop_back()
{
- assert(!_empty);
- _empty = _head == _tail;
- if (_tail == 0)
- --_round;
- decrease(_tail);
+ assert(!empty());
+ --_size;
}
/**
@@ -701,43 +496,45 @@
* @ingroup api_base_utils
*/
void
- push_back(typename Base::value_type val)
+ push_back(typename std::vector<T>::value_type val)
{
advance_tail();
- (*this)[_tail] = val;
+ back() = val;
}
/**
- * Increases the tail by one.
- * Check for wrap-arounds to update the round counter.
+ * Increases the tail by one. This may overwrite the head if the queue
is
+ * full.
*
* @ingroup api_base_utils
*/
void
advance_tail()
{
- increase(_tail);
- if (_tail == 0)
- ++_round;
-
- if (_tail == _head && !_empty)
- increase(_head);
-
- _empty = false;
+ if (full())
+ ++_head;
+ else
+ ++_size;
}
/**
- * Increases the tail by a specified number of steps
+ * Increases the tail by a specified number of steps. This may
overwrite
+ * one or more entries starting at the head if the queue is full.
*
* @param len Number of steps
*
* @ingroup api_base_utils
*/
void
- advance_tail(uint32_t len)
+ advance_tail(size_t len)
{
- for (auto idx = 0; idx < len; idx++)
- advance_tail();
+ size_t remaining = _capacity - _size;
+ if (len > remaining) {
+ size_t overflow = len - remaining;
+ _head += overflow;
+ len -= overflow;
+ }
+ _size += len;
}
/**
@@ -745,7 +542,7 @@
*
* @ingroup api_base_utils
*/
- bool empty() const { return _empty; }
+ bool empty() const { return _size == 0; }
/**
* Is the queue full?
@@ -755,28 +552,14 @@
*
* @ingroup api_base_utils
*/
- bool
- full() const
- {
- return !_empty &&
- (_tail + 1 == _head || (_tail + 1 == _capacity && _head == 0));
- }
+ bool full() const { return _size == _capacity; }
/**
* Iterators.
*
* @ingroup api_base_utils
*/
- iterator
- begin()
- {
- if (_empty)
- return end();
- else if (_head > _tail)
- return iterator(this, _head, _round - 1);
- else
- return iterator(this, _head, _round);
- }
+ iterator begin() { return iterator(this, _head); }
/* TODO: This should return a const_iterator. */
/**
@@ -785,28 +568,13 @@
iterator
begin() const
{
- if (_empty)
- return end();
- else if (_head > _tail)
- return iterator(const_cast<CircularQueue*>(this), _head,
- _round - 1);
- else
- return iterator(const_cast<CircularQueue*>(this), _head,
- _round);
+ return iterator(const_cast<CircularQueue*>(this), _head);
}
/**
* @ingroup api_base_utils
*/
- iterator
- end()
- {
- auto poi = moduloAdd(_tail, 1);
- auto round = _round;
- if (poi == 0)
- ++round;
- return iterator(this, poi, round);
- }
+ iterator end() { return iterator(this, tail() + 1); }
/**
* @ingroup api_base_utils
@@ -814,37 +582,11 @@
iterator
end() const
{
- auto poi = moduloAdd(_tail, 1);
- auto round = _round;
- if (poi == 0)
- ++round;
- return iterator(const_cast<CircularQueue*>(this), poi, round);
+ return iterator(const_cast<CircularQueue*>(this), tail() + 1);
}
- /**
- * Return an iterator to an index in the vector.
- * This poses the problem of round determination. By convention, the
round
- * is picked so that isValidIndex(idx, round) is true. If that is not
- * possible, then the round value is _round, unless _tail is at the
end of
- * the storage, in which case the PTE wraps up and becomes _round + 1
- */
- iterator
- getIterator(size_t idx)
- {
- assert(isValidIdx(idx) || moduloAdd(_tail, 1) == idx);
- if (_empty)
- return end();
-
- uint32_t round = _round;
- if (idx > _tail) {
- if (idx >= _head && _head > _tail) {
- round -= 1;
- }
- } else if (idx < _head && _tail + 1 == _capacity) {
- round += 1;
- }
- return iterator(this, idx, round);
- }
+ /** Return an iterator to an index in the queue. */
+ iterator getIterator(size_t idx) { return iterator(this, idx); }
};
#endif /* __BASE_CIRCULARQUEUE_HH__ */
diff --git a/src/base/circular_queue.test.cc
b/src/base/circular_queue.test.cc
index 51d3c01..ffbdce2 100644
--- a/src/base/circular_queue.test.cc
+++ b/src/base/circular_queue.test.cc
@@ -57,7 +57,7 @@
{
const auto cq_size = 8;
CircularQueue<uint32_t> cq(cq_size);
- ASSERT_EQ(cq.head(), cq.tail() + 1);
+ ASSERT_EQ(cq.head(), (cq.tail() + 1) % cq_size);
}
/** Adding elements to the circular queue.
@@ -135,7 +135,7 @@
}
ASSERT_TRUE(cq.full());
- ASSERT_EQ(cq.head(), cq.tail() + 1);
+ ASSERT_EQ(cq.head(), (cq.tail() + 1) % cq_size);
}
/** Testing CircularQueue::begin(), CircularQueue::end()
@@ -196,10 +196,8 @@
cq.push_back(first_value);
cq.push_back(second_value);
- auto negative_offset = -(cq_size + 1);
auto it_1 = cq.begin();
auto it_2 = cq.begin() + 1;
- auto it_3 = cq.begin() - negative_offset;
// Operators test
ASSERT_TRUE(it_1 != it_2);
@@ -213,7 +211,6 @@
ASSERT_EQ(it_1, it_2 - 1);
ASSERT_EQ(it_2 - it_1, 1);
ASSERT_EQ(it_1 - it_2, -1);
- ASSERT_EQ(it_3._round, 1);
auto temp_it = it_1;
ASSERT_EQ(++temp_it, it_2);
@@ -240,7 +237,7 @@
auto starting_it = cq.begin();
auto ending_it = starting_it + cq_size;
- ASSERT_EQ(starting_it._idx, ending_it._idx);
+ ASSERT_EQ(ending_it - starting_it, cq_size);
ASSERT_TRUE(starting_it != ending_it);
}
@@ -264,6 +261,5 @@
auto starting_it = cq.begin();
auto ending_it = cq.end();
- ASSERT_EQ(starting_it._round + 1, ending_it._round);
ASSERT_EQ(ending_it - starting_it, cq_size);
}
diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
index 9fe5b53..0b8a272 100644
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -223,11 +223,11 @@
uint8_t *memData;
/** Load queue index. */
- int16_t lqIdx;
+ ssize_t lqIdx;
LQIterator lqIt;
/** Store queue index. */
- int16_t sqIdx;
+ ssize_t sqIdx;
SQIterator sqIt;
diff --git a/src/cpu/o3/lsq_unit_impl.hh b/src/cpu/o3/lsq_unit_impl.hh
index 93ac009..2c6a16c 100644
--- a/src/cpu/o3/lsq_unit_impl.hh
+++ b/src/cpu/o3/lsq_unit_impl.hh
@@ -343,6 +343,7 @@
assert(!loadQueue.back().valid());
loadQueue.back().set(load_inst);
load_inst->lqIdx = loadQueue.tail();
+ assert(load_inst->lqIdx > 0);
load_inst->lqIt = loadQueue.getIterator(load_inst->lqIdx);
++loads;
@@ -400,7 +401,8 @@
storeQueue.advance_tail();
store_inst->sqIdx = storeQueue.tail();
- store_inst->lqIdx = loadQueue.moduloAdd(loadQueue.tail(), 1);
+ store_inst->lqIdx = loadQueue.tail() + 1;
+ assert(store_inst->lqIdx > 0);
store_inst->lqIt = loadQueue.end();
storeQueue.back().set(store_inst);
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38478
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: Ic2baa28de135be7086fa94579bbec451d69b3b15
Gerrit-Change-Number: 38478
Gerrit-PatchSet: 7
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Jason Lowe-Power <ja...@lowepower.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.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
