diff --git a/sparta/.gitignore b/sparta/.gitignore index 03004961cf..9378c4b050 100644 --- a/sparta/.gitignore +++ b/sparta/.gitignore @@ -11,3 +11,4 @@ build* [Ff]ast[Dd]ebug* cmake-build-* *~ +compile_commands.json \ No newline at end of file diff --git a/sparta/sparta/resources/Buffer.hpp b/sparta/sparta/resources/Buffer.hpp index f174c3d3c6..9f23785ea5 100644 --- a/sparta/sparta/resources/Buffer.hpp +++ b/sparta/sparta/resources/Buffer.hpp @@ -208,28 +208,32 @@ namespace sparta /// override the comparison operator. bool operator<(const BufferIterator& rhs) const { - sparta_assert(attached_buffer_ == rhs.attached_buffer_, "Cannot compare BufferIterators created by different buffers."); + sparta_assert(attached_buffer_ == rhs.attached_buffer_, + "Cannot compare BufferIterators created by different buffers."); return getIndex_() < rhs.getIndex_(); } /// override the comparison operator. bool operator>(const BufferIterator& rhs) const { - sparta_assert(attached_buffer_ == rhs.attached_buffer_, "Cannot compare BufferIterators created by different buffers."); + sparta_assert(attached_buffer_ == rhs.attached_buffer_, + "Cannot compare BufferIterators created by different buffers."); return getIndex_() > rhs.getIndex_(); } /// override the comparison operator. bool operator==(const BufferIterator& rhs) const { - sparta_assert(attached_buffer_ == rhs.attached_buffer_, "Cannot compare BufferIterators created by different buffers."); + sparta_assert(attached_buffer_ == rhs.attached_buffer_, + "Cannot compare BufferIterators created by different buffers."); return (buffer_entry_ == rhs.buffer_entry_); } /// override the not equal operator. bool operator!=(const BufferIterator& rhs) const { - sparta_assert(attached_buffer_ == rhs.attached_buffer_, "Cannot compare BufferIterators created by different buffers."); + sparta_assert(attached_buffer_ == rhs.attached_buffer_, + "Cannot compare BufferIterators created by different buffers."); return !operator==(rhs); } @@ -245,20 +249,23 @@ namespace sparta /// override the dereferencing operator DataReferenceType operator* () const { - sparta_assert(attached_buffer_, "The iterator is not attached to a buffer. Was it initialized?"); + sparta_assert(attached_buffer_, + "The iterator is not attached to a buffer. Was it initialized?"); sparta_assert(isValid(), "Iterator is not valid for dereferencing"); return *(buffer_entry_->data); } //! Overload the class-member-access operator. value_type * operator -> () { - sparta_assert(attached_buffer_, "The iterator is not attached to a buffer. Was it initialized?"); + sparta_assert(attached_buffer_, + "The iterator is not attached to a buffer. Was it initialized?"); sparta_assert(isValid(), "Iterator is not valid for dereferencing"); return buffer_entry_->data; } value_type const * operator -> () const { - sparta_assert(attached_buffer_, "The iterator is not attached to a buffer. Was it initialized?"); + sparta_assert(attached_buffer_, + "The iterator is not attached to a buffer. Was it initialized?"); sparta_assert(isValid(), "Iterator is not valid for dereferencing"); return buffer_entry_->data; } @@ -266,18 +273,15 @@ namespace sparta /** brief Move the iterator forward to point to next element in queue ; PREFIX */ BufferIterator & operator++() { - sparta_assert(attached_buffer_, "The iterator is not attached to a buffer. Was it initialized?"); - if(isValid()) { - uint32_t idx = buffer_entry_->physical_idx; - ++idx; - if(attached_buffer_->isValid(idx)) { - buffer_entry_ = attached_buffer_->buffer_map_[idx]; - } - else { - buffer_entry_ = nullptr; - } - } else { - sparta_assert(attached_buffer_->numFree() > 0, "Incrementing the iterator to entry that is not valid"); + sparta_assert(attached_buffer_, + "The iterator is not attached to a buffer. Was it initialized?"); + sparta_assert(isValid(), "Incrementing an iterator that is not valid"); + const uint32_t idx = buffer_entry_->physical_idx + 1; + if(attached_buffer_->isValid(idx)) { + buffer_entry_ = attached_buffer_->buffer_map_[idx]; + } + else { + buffer_entry_ = nullptr; } return *this; } @@ -438,7 +442,7 @@ namespace sparta */ const value_type & read(const const_reverse_iterator & entry) const { - return read(entry.base().getIndex_()); + return read(std::prev(entry.base())); } /** @@ -465,7 +469,7 @@ namespace sparta * \param entry the BufferIterator to read from. */ value_type & access(const const_reverse_iterator & entry) { - return access(entry.base().getIndex_()); + return access(std::prev(entry.base())); } /** @@ -616,10 +620,11 @@ namespace sparta * a BufferIterator has been created, the * erase(BufferIterator&) should be used. */ - void erase(const uint32_t& idx) + void erase(uint32_t idx) { // Make sure we are invalidating an already valid object. - sparta_assert(idx < size(), "Cannot erase an index that is not already valid"); + sparta_assert(idx < size(), + "Cannot erase an index that is not already valid"); // Do the invalidation immediately // 1. Move the free space pointer to the erased position. @@ -634,19 +639,18 @@ namespace sparta validator_->detachDataPointer(free_position_); // Shift all the positions above the invalidation in the map one space down. - uint32_t i = idx; sparta_assert(num_valid_ > 0); const uint32_t top_idx_of_buffer = num_valid_ - 1; - while(i < top_idx_of_buffer) + while(idx < top_idx_of_buffer) { // assert that we are not going to do an invalid read. - sparta_assert(i + 1 < num_entries_); - buffer_map_[i] = buffer_map_[i + 1]; - buffer_map_[i]->physical_idx = i; + sparta_assert(idx + 1 < num_entries_); + buffer_map_[idx] = buffer_map_[idx + 1]; + buffer_map_[idx]->physical_idx = idx; // Shift the indexes in the address map. - address_map_[i] = address_map_[i + 1]; - ++i; + address_map_[idx] = address_map_[idx + 1]; + ++idx; } // the entry at the old num_valid_ in the map now points to nullptr @@ -664,22 +668,22 @@ namespace sparta * \brief erase the index at which the entry exists in the Buffer. * \param entry a reference to the entry to be erased. */ - void erase(const const_iterator& entry) + iterator erase(const const_iterator& entry) { - sparta_assert(entry.attached_buffer_ == this, "Cannot erase an entry created by another Buffer"); + sparta_assert(entry.attached_buffer_ == this, + "Cannot erase an entry created by another Buffer"); // erase the index in the actual buffer. erase(entry.getIndex_()); + return {this, buffer_map_[entry.getIndex_()]}; } /** * \brief erase the index at which the entry exists in the Buffer. * \param entry a reference to the entry to be erased. */ - void erase(const const_reverse_iterator& entry) + reverse_iterator erase(const const_reverse_iterator& entry) { - sparta_assert(entry.base().attached_buffer_ == this, "Cannot erase an entry created by another Buffer"); - // erase the index in the actual buffer. - erase(entry.base().getIndex_()); + return reverse_iterator{erase(std::prev(entry.base()))}; } /** @@ -871,7 +875,8 @@ namespace sparta void resizeInternalContainers_() { // Assert that the Buffer class is in Infinite-Mode. - sparta_assert(is_infinite_mode_, "The Buffer class must be in Infinite-Mode in order to resize itself."); + sparta_assert(is_infinite_mode_, + "The Buffer class must be in Infinite-Mode in order to resize itself."); // We do not resize if there are available slots in buffer. if(numFree() != 0) { @@ -998,14 +1003,15 @@ namespace sparta std::string name_; const Clock * clk_ = nullptr; - size_type num_entries_; /*!< The number of entries this buffer can hold */ - PointerList buffer_map_; /*!< A vector list of pointers to all the items active in the buffer */ - size_type data_pool_size_; /*!< The number of elements our data_pool_ can hold*/ - DataPool data_pool_; /*!< A vector twice the size of our Buffer size limit that is filled with pointers for our data.*/ - - DataPointer* free_position_ = 0; /*!< A pointer to a free position in our data_pool_ */ - DataPointer* first_position_ = 0; /*!< A pointer to a first position in our data_pool_; used for lower bound check */ - size_type num_valid_ = 0; /*!< A tally of valid items */ + size_type num_entries_ = 0; /*!< The number of entries this buffer can hold */ + PointerList buffer_map_; /*!< A vector list of pointers to all the items active in the buffer */ + size_type data_pool_size_ = 0; /*!< The number of elements our data_pool_ can hold*/ + DataPool data_pool_; /*!< A vector twice the size of our Buffer size limit + that is filled with pointers for our data.*/ + + DataPointer* free_position_ = nullptr; /*!< A pointer to a free position in our data_pool_ */ + DataPointer* first_position_ = nullptr; /*!< A pointer to a first position in our data_pool_; used for lower bound check */ + size_type num_valid_ = 0; /*!< A tally of valid items */ std::unique_ptr validator_; /*!< Checks the validity of DataPointer */ ////////////////////////////////////////////////////////////////////// diff --git a/sparta/test/Buffer/Buffer_test.cpp b/sparta/test/Buffer/Buffer_test.cpp index c80a8a628b..66d24b61a7 100644 --- a/sparta/test/Buffer/Buffer_test.cpp +++ b/sparta/test/Buffer/Buffer_test.cpp @@ -138,6 +138,8 @@ void generalTest() EXPECT_TRUE(dummy_3.s_field.size() == 0); EXPECT_TRUE(buf_dummy.read(0).s_field == "GHI"); auto ritr = buf_dummy.rbegin(); + EXPECT_TRUE(ritr->s_field == "ABC"); + EXPECT_TRUE(buf_dummy.read(ritr).s_field == "ABC"); buf_dummy.insert(++ritr, std::move(dummy_4)); EXPECT_TRUE(dummy_4.s_field.size() == 0); EXPECT_TRUE(buf_dummy.read(2).s_field == "JKL"); @@ -497,7 +499,8 @@ void generalTest() itr = buf10.end(); - EXPECT_THROW_MSG_CONTAINS(++itr, "Incrementing the iterator to entry that is not valid"); + EXPECT_THROW_MSG_CONTAINS(++itr, + "Incrementing an iterator that is not valid"); itr = buf10.end(); --itr; @@ -517,6 +520,7 @@ void generalTest() // REVERSE_ITERATOR tests buf10.clear(); + EXPECT_TRUE(buf10.rbegin() == buf10.rend()); EXPECT_EQUAL(buf10.size(), 0); for(uint32_t i = 0; i < 9; ++i) { buf10.push_back(20.5 + i); @@ -525,33 +529,34 @@ void generalTest() // Check for correct response when using the increment operator auto ritr = buf10.rbegin(); - ++ritr; EXPECT_EQUAL(buf10.read(ritr), 1234.5); + ++ritr; + EXPECT_EQUAL(buf10.read(ritr), 28.5); ritr = buf10.rend(); EXPECT_THROW_MSG_CONTAINS(++ritr, "Decrementing the iterator results in buffer underrun"); ritr = buf10.rend(); --ritr; - EXPECT_EQUAL(buf10.read(ritr), 21.5); - ++ritr; EXPECT_EQUAL(buf10.read(ritr), 20.5); + ++ritr; + EXPECT_THROW_MSG_CONTAINS(buf10.read(ritr), "Decrementing the iterator results in buffer underrun"); ritr = buf10.rbegin(); EXPECT_EQUAL(*ritr, 1234.5); ++ritr; - // This looks wrong... - EXPECT_EQUAL(buf10.access(ritr), 1234.5); - EXPECT_EQUAL(buf10.read(ritr), 1234.5); + EXPECT_EQUAL(buf10.access(ritr), 28.5); + EXPECT_EQUAL(buf10.read(ritr), 28.5); buf10.erase(9); buf10.erase(8); buf10.erase(7); EXPECT_EQUAL(buf10.size(), 7); - EXPECT_THROW_MSG_SHORT(buf10.read(ritr),"isValid(idx)"); - ++ritr; // should point to 6 EXPECT_EQUAL(buf10.read(ritr), 26.5); + ++ritr; // should point to 5 + EXPECT_EQUAL(buf10.read(ritr), 25.5); --ritr; // What should this do? + EXPECT_TRUE(ritr == buf10.rbegin()); sched.run(1); @@ -564,12 +569,14 @@ void generalTest() buf10.push_back(1234.5); ritr = buf10.rbegin(); - EXPECT_THROW_MSG_CONTAINS(--ritr, "Incrementing the iterator to entry that is not valid"); + EXPECT_THROW_MSG_CONTAINS(--ritr, + "Incrementing an iterator that is not valid"); ritr = buf10.rbegin(); ++ritr; - EXPECT_EQUAL(buf10.read(ritr), 1234.5); + EXPECT_EQUAL(buf10.read(ritr), 28.5); --ritr; + EXPECT_EQUAL(buf10.read(ritr), 1234.5); sched.run(5); @@ -707,6 +714,104 @@ void testPointerTypes() EXPECT_TRUE(ptr != nullptr); } +template +void testEraseSupport() +{ + BuffType int_buff("simple int buff", 10, nullptr); + + for(uint32_t i = 0; i < int_buff.capacity(); ++i) { + int_buff.push_back(i + 100); + } + EXPECT_EQUAL(int_buff.size(), int_buff.capacity()); + + IterType it; + IterType eit; + if constexpr(std::is_same_v) { + it = int_buff.rbegin(); + eit = int_buff.rend(); + } + else { + it = int_buff.begin(); + eit = int_buff.end(); + } + + while(it != eit) { + it = int_buff.erase(it); + } + EXPECT_EQUAL(0, int_buff.size()); + + // Pure fill, partial erase + for(uint32_t i = 0; i < int_buff.capacity(); ++i) { + int_buff.push_back(i + 100); + } + + if constexpr(std::is_same_v) { + it = int_buff.rbegin(); + eit = int_buff.rend(); + } + else { + it = int_buff.begin(); + eit = int_buff.end(); + } + while(it != eit) { + if(*it % 2 == 0) { + it = int_buff.erase(it); + } + else { + ++it; + } + } + EXPECT_EQUAL(5, int_buff.size()); + int_buff.clear(); + + // Pure fill, erase 1, add 1 erase, another add another + for(uint32_t i = 0; i < int_buff.capacity(); ++i) { + int_buff.push_back(i + 100); + } + + // Pattern: 100 -> 109 + + // Erase 105 + if constexpr(std::is_same_v) { + it = int_buff.rbegin(); + eit = int_buff.rend(); + } + else { + it = int_buff.begin(); + eit = int_buff.end(); + } + while(it != eit) { + if(*it == 105) { + it = int_buff.erase(it); + EXPECT_EQUAL(*it, 106); + break; + } + ++it; + } + + // Put in 200 + int_buff.push_back(200); + EXPECT_EQUAL(int_buff.accessBack(), 200); + + std::array vals = {100, 101, 102, 103, 104, 106, 107, 108, 109, 200}; + + uint32_t idx = 0; + if constexpr(std::is_same_v) { + it = int_buff.rbegin(); + eit = int_buff.rend(); + } + else { + it = int_buff.begin(); + eit = int_buff.end(); + } + while(it != eit) { + EXPECT_EQUAL(vals[idx++], *it); + it = int_buff.erase(it); + } + + EXPECT_EQUAL(0, int_buff.size()); +} + void testInvalidates() { EXPECT_EQUAL(dummy_allocs, 0); @@ -750,6 +855,12 @@ void testInvalidates() EXPECT_EQUAL(SimpleStruct::simple_allocs, 1); my_simple_struct.erase(my_simple_struct.begin()); EXPECT_EQUAL(SimpleStruct::simple_allocs, 0); + + // Pure fill, pure erase + testEraseSupport::iterator, sparta::Buffer>(); + testEraseSupport::const_iterator, sparta::Buffer>(); + //testEraseSupport::reverse_iterator, sparta::Buffer>(); + // testEraseSupport::const_reverse_iterator, sparta::Buffer>(); } diff --git a/sparta/test/Register/Register_test.cpp b/sparta/test/Register/Register_test.cpp index 276cd13533..b6a3189c9f 100644 --- a/sparta/test/Register/Register_test.cpp +++ b/sparta/test/Register/Register_test.cpp @@ -267,7 +267,7 @@ void testFieldRegisterWrite() }; RootTreeNode root; DummyDevice good_dummy(&root); - std::unique_ptr regs(RegisterSet::create(&good_dummy, good_reg_defs)); + std::unique_ptr regs = RegisterSet::create(&good_dummy, good_reg_defs); regs->getRegister("fp_reg")->getField("sp")->write(1); regs->getRegister("fp_reg")->getField("dp")->write(1); @@ -304,7 +304,7 @@ void testGoodRegs() RootTreeNode root; DummyDevice good_dummy(&root); - std::unique_ptr good_regs(RegisterSet::create(&good_dummy, good_reg_defs)); + std::unique_ptr good_regs = RegisterSet::create(&good_dummy, good_reg_defs); #ifndef REGISTER_SET_GET_ARCH_DATA_REMOVED EXPECT_TRUE(good_regs->getArchData().isLaidOut()); std::cout << "Layout of good dummy regs:" << std::endl; @@ -759,8 +759,8 @@ int main() root.bindTreeLate(); // Construct some good and bad regs to test out size constraints - testFieldRegisterWrite(); - testGoodRegs(); + testFieldRegisterWrite(); // Create registers directly + testGoodRegs(); // Create registers directly testBadRegs();