panfrost,lima: use index size in panfrost minmax_cache

Bifrost keeps a cache of information about buffers being
used as indices. Unfortunately, it was not keeping information
about the size of the indices (probably because this rarely
changes). If a program deliberately re-interprets the indices
as a different type (e.g. UNSIGNED_INT instead of UNSIGNED_SHORT)
then we will use incorrect values from the cache. This actually
showed up in a test program we were running.

Fix by saving the index size in the cache key.

Cc: mesa-stable
Reviewed-by: Erik Faye-Lund <erik.faye-lund@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34011>
This commit is contained in:
Eric R. Smith 2025-03-11 18:56:34 -03:00 committed by Marge Bot
parent 6fab29d37e
commit 739da17f6e
6 changed files with 90 additions and 22 deletions

View file

@ -1091,14 +1091,16 @@ lima_draw_vbo_indexed(struct pipe_context *pctx,
else {
ctx->index_res = lima_resource(info->index.resource);
ctx->index_offset = 0;
needs_indices = !panfrost_minmax_cache_get(ctx->index_res->index_cache, draw->start,
draw->count, &ctx->min_index, &ctx->max_index);
needs_indices = !panfrost_minmax_cache_get(ctx->index_res->index_cache, info->index_size,
draw->start, draw->count,
&ctx->min_index, &ctx->max_index);
}
if (needs_indices) {
u_vbuf_get_minmax_index(pctx, info, draw, &ctx->min_index, &ctx->max_index);
if (!info->has_user_indices)
panfrost_minmax_cache_add(ctx->index_res->index_cache, draw->start, draw->count,
panfrost_minmax_cache_add(ctx->index_res->index_cache, info->index_size,
draw->start, draw->count,
ctx->min_index, ctx->max_index);
}

View file

@ -708,7 +708,9 @@ lima_transfer_map(struct pipe_context *pctx,
ptrans->layer_stride = res->levels[level].layer_stride;
if ((usage & PIPE_MAP_WRITE) && (usage & PIPE_MAP_DIRECTLY))
panfrost_minmax_cache_invalidate(res->index_cache, ptrans->box.x, ptrans->box.width);
panfrost_minmax_cache_invalidate(res->index_cache,
util_format_get_blocksize(pres->format),
ptrans->box.x, ptrans->box.width);
return bo->map + res->levels[level].offset +
box->z * res->levels[level].layer_stride +
@ -817,7 +819,9 @@ lima_transfer_unmap(struct pipe_context *pctx,
if (trans->staging)
free(trans->staging);
if (ptrans->usage & PIPE_MAP_WRITE) {
panfrost_minmax_cache_invalidate(res->index_cache, ptrans->box.x, ptrans->box.width);
panfrost_minmax_cache_invalidate(res->index_cache,
util_format_get_blocksize(res->base.format),
ptrans->box.x, ptrans->box.width);
}
pipe_resource_reference(&ptrans->resource, NULL);

View file

@ -141,7 +141,8 @@ panfrost_get_index_buffer_bounded(struct panfrost_batch *batch,
} else if (!info->has_user_indices) {
/* Check the cache */
needs_indices = !panfrost_minmax_cache_get(
rsrc->index_cache, draw->start, draw->count, min_index, max_index);
rsrc->index_cache, info->index_size, draw->start, draw->count,
min_index, max_index);
}
if (needs_indices) {
@ -149,7 +150,8 @@ panfrost_get_index_buffer_bounded(struct panfrost_batch *batch,
u_vbuf_get_minmax_index(&ctx->base, info, draw, min_index, max_index);
if (!info->has_user_indices)
panfrost_minmax_cache_add(rsrc->index_cache, draw->start, draw->count,
panfrost_minmax_cache_add(rsrc->index_cache, info->index_size,
draw->start, draw->count,
*min_index, *max_index);
}

View file

@ -1431,7 +1431,9 @@ panfrost_ptr_map(struct pipe_context *pctx, struct pipe_resource *resource,
if (usage & PIPE_MAP_WRITE) {
BITSET_SET(rsrc->valid.data, level);
panfrost_minmax_cache_invalidate(
rsrc->index_cache, transfer->base.box.x, transfer->base.box.width);
rsrc->index_cache,
util_format_get_blocksize(rsrc->base.format),
transfer->base.box.x, transfer->base.box.width);
}
return bo->ptr.cpu + rsrc->image.layout.slices[level].offset +
@ -1892,7 +1894,9 @@ panfrost_ptr_unmap(struct pipe_context *pctx, struct pipe_transfer *transfer)
transfer->box.x + transfer->box.width);
if (transfer->usage & PIPE_MAP_WRITE) {
panfrost_minmax_cache_invalidate(prsrc->index_cache, transfer->box.x,
panfrost_minmax_cache_invalidate(prsrc->index_cache,
util_format_get_blocksize(prsrc->base.format),
transfer->box.x,
transfer->box.width);
}

View file

@ -41,17 +41,59 @@
#include "pan_minmax_cache.h"
#include "util/macros.h"
bool
panfrost_minmax_cache_get(struct panfrost_minmax_cache *cache, unsigned start,
unsigned count, unsigned *min_index,
unsigned *max_index)
/*
* note: a count of 0 would be an empty range, which we don't have to
* cache; so returning a 0 to indicate "do not cache" is sensible
* otherwise create a key that encodes the start, count, and index size
*/
static uint64_t
panfrost_calc_cache_key(struct panfrost_minmax_cache *cache, unsigned index_size,
unsigned start, unsigned count)
{
uint64_t ht_key = (((uint64_t)count) << 32) | start;
bool found = false;
uint64_t ht_key;
if (!cache)
return 0; /* do not cache if no cache! */
/* we're going to store the item size in the upper bits of the count;
* if the count is too big to do this safely, bail and do not use
* the cache (this case is going to be horrible no matter what we do,
* and is highly unlikely)
*/
if (count > 0x3FFFFFFF)
return 0; /* do not cache */
/* find log2(index_size) or die tryin' */
switch (index_size) {
case 1:
index_size = 0;
break;
case 2:
index_size = 1;
break;
case 4:
index_size = 2;
break;
default:
unreachable("unknown index size");
}
count = count | (index_size << 30);
ht_key = ((uint64_t)count << 32) | start;
return ht_key;
}
bool
panfrost_minmax_cache_get(struct panfrost_minmax_cache *cache, unsigned index_size,
unsigned start, unsigned count,
unsigned *min_index, unsigned *max_index)
{
uint64_t ht_key = panfrost_calc_cache_key(cache, index_size, start, count);
bool found = false;
if (!ht_key)
return false;
for (unsigned i = 0; i < cache->size; ++i) {
if (cache->keys[i] == ht_key) {
uint64_t hit = cache->values[i];
@ -67,15 +109,15 @@ panfrost_minmax_cache_get(struct panfrost_minmax_cache *cache, unsigned start,
}
void
panfrost_minmax_cache_add(struct panfrost_minmax_cache *cache, unsigned start,
unsigned count, unsigned min_index,
unsigned max_index)
panfrost_minmax_cache_add(struct panfrost_minmax_cache *cache, unsigned index_size,
unsigned start, unsigned count,
unsigned min_index, unsigned max_index)
{
uint64_t ht_key = (((uint64_t)count) << 32) | start;
uint64_t ht_key = panfrost_calc_cache_key(cache, index_size, start, count);
uint64_t value = min_index | (((uint64_t)max_index) << 32);
unsigned index = 0;
if (!cache)
if (!ht_key)
return;
if (cache->size == PANFROST_MINMAX_SIZE) {
@ -95,19 +137,30 @@ panfrost_minmax_cache_add(struct panfrost_minmax_cache *cache, unsigned start,
void
panfrost_minmax_cache_invalidate(struct panfrost_minmax_cache *cache,
unsigned index_size,
size_t offset, size_t size)
{
/* Ensure there is a cache to invalidate and a write */
if (!cache)
return;
/* convert offset and size to bytes, so that if we
update a region using a different item size we
still invalidate it */
offset *= index_size;
size *= index_size;
unsigned valid_count = 0;
for (unsigned i = 0; i < cache->size; ++i) {
uint64_t key = cache->keys[i];
uint32_t start = key & 0xffffffff;
uint32_t count = key >> 32;
/* the item size is in the upper 2 bits of the key
* as above, convert size and count to bytes to make
* region comparison agnostic to item size
*/
uint32_t key_index_size = (key >> 62);
size_t count = ((key >> 32) & 0x3fffffff) << key_index_size;
size_t start = (key & 0xffffffff) << key_index_size;
/* 1D range intersection */
bool invalid = MAX2(offset, start) <

View file

@ -41,14 +41,17 @@ struct panfrost_minmax_cache {
};
bool panfrost_minmax_cache_get(struct panfrost_minmax_cache *cache,
unsigned index_size,
unsigned start, unsigned count,
unsigned *min_index, unsigned *max_index);
void panfrost_minmax_cache_add(struct panfrost_minmax_cache *cache,
unsigned index_size,
unsigned start, unsigned count,
unsigned min_index, unsigned max_index);
void panfrost_minmax_cache_invalidate(struct panfrost_minmax_cache *cache,
unsigned index_size,
size_t offset, size_t size);
#endif