Skip to content

New drcachesim replacement policy code passes wrong values for set count #7672

@derekbruening

Description

@derekbruening

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_t> cache_lru_test(/*associativity=*/4,
                                                         /*line_size=*/32);
    cache_lru_test.initialize_cache(std::unique_ptr<policy_lru_t>(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.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions