DynamoRIO / DynamoRIO/dynamorio

New drcachesim replacement policy code passes wrong values for set count

Open
#7,672 0 comments 0 reactions 1 assignee Claimed by @oralmer View on GitHub
Bug-ToolFail Component-DrMemtrace
Dominant language
C
Stars
3.2k
Forks
629
Avg merge
2d 15h
Merged PRs (30d)
31

Description

PR #7314 refactoring the cache replacement code to add a new interface cache_replacement_policy_t.
The cache_replacement_policy_t class takes in the number of sets and the associativity.
But the number of sets passed in seems incorrect in multiple places:

When cache_simulator_t creates caches it passes the total line count instead of the lines per way:
```
create_cache_replacement_policy(
knobs_.replace_policy, (int)knobs_.LL_size / (int)knobs_.line_size,
```

Further confusing things is that caching_device_t takes in the total line count==block count, but the replacement policy does seem to want the count per way, as policies like LRU expect associativity per set:

```
// Initialize the LRU list for each set.
lru_counters_.reserve(num_sets);
for (int i = 0; i < num_sets; ++i) {
lru_counters_.emplace_back(associativity, 1);
}
```

The TLB simulator seems correct since there is no "line size" and the total size is in entry counts:
```
auto replace_policy = create_cache_replacement_policy(
knobs_.TLB_replace_policy, knobs_.TLB_L1I_entries / knobs_.TLB_L1I_assoc,
```

Some new tests also seem incorrect, in a different way from the cache_simulator_t:
```
void
unit_test_cache_lru_four_way()
{
caching_device_policy_test_t cache_lru_test(/*associativity=*/4,
/*line_size=*/32);
cache_lru_test.initialize_cache(std::unique_ptr(new policy_lru_t(
/*num_blocks=*/256 / 4, /*associativity=*/4)),
256);
```
That test says cache size is 256 bytes, line size is 32 bytes, assoc is 4. That should mean 2 lines == sets for each of the 4 ways. But it passes num_sets to policy_lru_t (calling it num_blocks) as 256/4 == 64.

Most likely these tests do not depend on the set count and are only really checking replacement within a particular line, so this was not detected by the tests.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.