DynamoRIO / dynamorio

Dynamic Instrumentation Tool Platform
Other
2.61k stars 554 forks source link

LL cache contains empty children vector in the default configuration #5031

Open mat-kalinowski opened 3 years ago

mat-kalinowski commented 3 years ago

Describe the bug In the default programmatic configuration using constructor cache_simulator_t(const cache_simulator_knobs_t &knobs) LL cache is created with empty children_ vector. init function with the default value for the children_ vector is used:

bool
init(int associativity, int line_size, int total_size, caching_device_t *parent,
      ...
      const std::vector<caching_device_t *> &children = {})

Probably it should contain L1i and L1d cache in this vector. With the same cache hierarchy configured through the file (using constructor cache_simulator_t(std::istream *config_file)) corresponding cache is created with proper values in the children_ vector.

Because of this behavior invalidates are probably not counted properly in the default configuration. There is a lot of code like this in the caching_device_t class:

if (!children_.empty() && inclusive_) {
    for (auto &child : children_) {
        child->invalidate(victim_tag, INVALIDATION_INCLUSIVE);
    }
}

To Reproduce

Run drcachesim in the default configuration without configuration file. Invalidates in L1d and L1i are always zero in the results:

Cache simulation results:
Core #0 (1 thread(s))
  L1I stats:
    Hits:                        9,471,930
    Misses:                         13,534
    Invalidations:                       0
    Miss rate:                        0.14%
  L1D stats:
    Hits:                        3,081,738
    Misses:                         43,079
    Invalidations:                       0
    Prefetch hits:                   8,082
    Prefetch misses:                34,997
    Miss rate:                        1.38%
LL stats:
    Hits:                           36,282
    Misses:                         20,331
    Invalidations:                       0
    Prefetch hits:                  20,056
    Prefetch misses:                14,941
    Local miss rate:                 35.91%
    Child hits:                 12,561,750
    Total miss rate:                  0.16%

The same app tested with the same cache hierarchy in the configuration file gives non-zero invalidates number in L1d and L1i.

Expected behavior I think LL cache in the default configuration should contain L1i and L1d cache in the children vector. Constructor cache_simulator_t(const cache_simulator_knobs_t &knobs) should invoke caching_device_t::init with proper value provided for LL cache instead of using default value.

Versions I was checking latest code from the master branch.

abhinav92003 commented 3 years ago

Thanks for filing the issue. For the configuration generated in cache_simulator_t(const cache_simulator_knobs_t &knobs), I think it makes sense to include L1I and L1D caches as children of the LLC. Can you also create a pull request for this?

mat-kalinowski commented 3 years ago

Sure, I will implement the fix and create pull request.