Skip to content

Commit

Permalink
Fixed bug with cache line dirty flag being reset. (ChampSim#389)
Browse files Browse the repository at this point in the history
Cache-lines could potentially have their dirty flag reset if a read-like
access was made before write-back/eviction.
Along with this, a test was created to ensure proper cache write-back
for lines that should be marked dirty.
  • Loading branch information
maccoymerrell authored Sep 20, 2023
1 parent 240b8d2 commit b44625f
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ bool CACHE::try_hit(const tag_lookup_type& handle_pkt)
for (auto ret : handle_pkt.to_return)
ret->push_back(response);

way->dirty = (handle_pkt.type == access_type::WRITE);
way->dirty |= (handle_pkt.type == access_type::WRITE);

// update prefetch stats and reset prefetch bit
if (useful_prefetch) {
Expand Down
105 changes: 105 additions & 0 deletions test/cpp/src/407-writeback-dirty.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
#include <catch.hpp>
#include "mocks.hpp"
#include "defaults.hpp"
#include "cache.h"
#include "champsim_constants.h"

SCENARIO("Blocks that have been written are marked dirty") {
GIVEN("An empty cache") {
constexpr uint64_t hit_latency = 4;
constexpr uint64_t miss_latency = 3;
do_nothing_MRC mock_ll;
to_wq_MRP mock_ul_seed;
to_rq_MRP mock_ul_test;
CACHE uut{CACHE::Builder{champsim::defaults::default_l2c}
.name("407-uut")
.sets(1)
.ways(1)
.upper_levels({{&mock_ul_seed.queues, &mock_ul_test.queues}})
.lower_level(&mock_ll.queues)
.hit_latency(hit_latency)
.fill_latency(miss_latency)
};

std::array<champsim::operable*, 4> elements{{&uut, &mock_ll, &mock_ul_seed, &mock_ul_test}};

for (auto elem : elements) {
elem->initialize();
elem->warmup = false;
elem->begin_phase();
}

// Run the uut for a few cycles
for (auto i = 0; i < 10; ++i)
for (auto elem : elements)
elem->_operate();

WHEN("A packet is sent") {
uint64_t id = 1;
decltype(mock_ul_seed)::request_type seed_a;
seed_a.address = 0xdeadbeef;
seed_a.cpu = 0;
seed_a.type = access_type::WRITE;
seed_a.instr_id = id++;

auto seed_a_result = mock_ul_seed.issue(seed_a);

for (uint64_t i = 0; i < 2*(miss_latency+hit_latency); ++i)
for (auto elem : elements)
elem->_operate();

THEN("The issue is received") {
CHECK(seed_a_result);
CHECK(mock_ll.packet_count() == 0);
}

AND_WHEN("The same packet is read") {
auto seed_b = seed_a;
seed_b.type = access_type::LOAD;
seed_b.instr_id = id++;

auto seed_b_result = mock_ul_test.issue(seed_b);

for (uint64_t i = 0; i < 2*(miss_latency+hit_latency); ++i)
for (auto elem : elements)
elem->_operate();

THEN("The issue is received") {
CHECK(seed_b_result);
CHECK(mock_ll.packet_count() == 0);
}

AND_WHEN("A packet with a different address is sent") {
decltype(mock_ul_test)::request_type test;
test.address = 0xcafebabe;
test.cpu = 0;
test.type = access_type::LOAD;
test.instr_id = id++;

auto test_b_result = mock_ul_test.issue(test);

for (uint64_t i = 0; i < hit_latency+1; ++i)
for (auto elem : elements)
elem->_operate();

THEN("The issue is received") {
CHECK(test_b_result);
REQUIRE_THAT(mock_ll.addresses, Catch::Matchers::RangeEquals(std::array{test.address}));
}

for (uint64_t i = 0; i < 2*(miss_latency+hit_latency); ++i)
for (auto elem : elements)
elem->_operate();

THEN("It takes exactly the specified cycles to return") {
REQUIRE(mock_ul_test.packets.back().return_time == mock_ul_test.packets.back().issue_time + (miss_latency + hit_latency + 1));
}

THEN("The first block is evicted") {
REQUIRE_THAT(mock_ll.addresses, Catch::Matchers::RangeEquals(std::array{test.address, seed_a.address}));
}
}
}
}
}
}

0 comments on commit b44625f

Please sign in to comment.