i#7291: support large pages in TLB #7292
Conversation
Changed `block_size` to `long int` in order to allow larger pages in TLB. Also added two minor fixes that came up while working on the feature: - Argument name mismatches in `cache_lru_t` and `cache_fifo_t` between the header and body. - Clearer error messages in `tlb_simulator_t`, so you can know exactly what failed to build. Fixes #7291
| @@ -74,7 +74,7 @@ caching_device_t::~caching_device_t() | |||
| } | |||
|
|
|||
| bool | |||
| caching_device_t::init(int associativity, int block_size, int num_blocks, | |||
| caching_device_t::init(int associativity, long int block_size, int num_blocks, | |||
There was a problem hiding this comment.
Should we widen num_blocks while at it or we are certain 32-bit is plenty?
There was a problem hiding this comment.
for my current needs it's enough, but I do think we can have a conversation about this. IMO:
uint64_t associativity // describes the size of a data structure
uint8_t lg2_block_size // we only allow powers of 2 and only use this int to calc log2
uint64_t num_blocks // describes the size of a data structure
uint64_t total_size // in cache_t
but these changes would lead to an interface change, that I'd probably like to avoid. we can also change the ints to int64_t and leave it at that, any fixes from this change will be very simple. WDYT?
There was a problem hiding this comment.
@brettcoon any opinions here?
There was a problem hiding this comment.
I favor signed over unsigned except when unsigned is explicitly needed (e.g. bitmasks). I'm not sure why anybody needs 64 bits for associativity, but I do agree that 64 bits seems appropriate for block_size, num_blocks, and total_size. So I like int64_t for those three.
|
Note that this Alpine job broke separately and is now fixed in PR #7289. |
oralmer
changed the title
Samll fixes which were not enough to justify a seperate issue. - Fixed inconsistent argument names in cache_fifo and cach_lru - header vs implementation. - Improved error printing in tlb_t so you can know what actually failed - Added and `add_parent` method to `caching_device_t`
Changed
block_sizetolong intin order to allow larger pages in TLB. Also added two minor fixes that came up while working on the feature:cache_lru_tandcache_fifo_tbetween the header and body.tlb_simulator_t, so you can know exactly what failed to build.Fixes #7291