base: Fix CircularQueue when diffing iterators

This patch is fixing CircularQueue iterators' subtraction, in particular
the behaviour when head and tail round multiple times.

Change-Id: Ie79ac8accd30a10cf039cf4def87675b01375d6b
Signed-off-by: Giacomo Travaglini <giacomo.travaglini@arm.com>
Reviewed-by: Gabor Dozsa <gabor.dozsa@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/17188
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Reviewed-by: Jason Lowe-Power <jason@lowepower.com>
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>

diff --git a/src/base/circular_queue.hh b/src/base/circular_queue.hh
index 5023661b91a4d1ef8bfd7bb294cde87dcaded33e..f368ed0d1efe4d51220e1466f5c7fea5c0ddbbf9 100644 (file)
--- a/src/base/circular_queue.hh
+++ b/src/base/circular_queue.hh
@@ -106,10 +106,19 @@ class CircularQueue : private std::vector<T>
     static uint32_t
     moduloSub(uint32_t op1, uint32_t op2, uint32_t size)
     {
-        int ret = (uint32_t)(op1 - op2) % 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);
@@ -355,10 +364,10 @@ class CircularQueue : private std::vector<T>
         difference_type operator-(const iterator& that)
         {
             /* If a is already at the end, we can safely return 0. */
-            auto ret = _cq->moduloSub(this->_idx, that._idx);
+            auto ret = _cq->sub(this->_idx, that._idx, _cq->capacity());
 
-            if (ret == 0 && this->_round != that._round) {
-                ret += this->_round * _cq->capacity();
+            if (this->_round != that._round) {
+                ret += ((this->_round - that._round) * _cq->capacity());
             }
             return ret;
         }

diff --git a/src/base/circular_queue.test.cc b/src/base/circular_queue.test.cc
index db59c3049796573af93255f97ffa0db8c60863af..cce6cb0b6eae1779aad6706d07206b687cf93ad8 100644 (file)
--- a/src/base/circular_queue.test.cc
+++ b/src/base/circular_queue.test.cc
@@ -212,6 +212,7 @@ TEST(CircularQueueTest, IteratorsOp)
     ASSERT_EQ(it_1 + 1, it_2);
     ASSERT_EQ(it_1, it_2 - 1);
     ASSERT_EQ(it_2 - it_1, 1);
+    ASSERT_EQ(it_1 - it_2, -1);
 
     auto temp_it = it_1;
     ASSERT_EQ(++temp_it, it_2);
@@ -241,3 +242,27 @@ TEST(CircularQueueTest, FullLoop)
     ASSERT_EQ(starting_it._idx, ending_it._idx);
     ASSERT_TRUE(starting_it != ending_it);
 }
+
+/**
+ * Testing correct behaviour when rounding multiple times:
+ * - Round indexes in sync
+ * - Difference between begin() and end() iterator is still
+ * equal to the CircularQueue size.
+ */
+TEST(CircularQueueTest, MultipleRound)
+{
+    const auto cq_size = 8;
+    CircularQueue<uint32_t> cq(cq_size);
+
+    // Filling the queue making it round multiple times
+    auto items_added = cq_size * 3;
+    for (auto idx = 0; idx < items_added; idx++) {
+        cq.push_back(0);
+    }
+
+    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);
+}