nir/lower_printf: hash format strings in nir_printf_fmt

Lionel added a neat debugging tool. Let's make it work with the new-style
hashing approach too, since nir_printf_fmt is a lot more convenient than needing
to define a dedicated CL function to access printf (although that works too).

We remove the old non-hashed path, because it has no more functional users --
hashing is a hard requirement with vtn_bindgen2, which Intel has now switched
to.

Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33380>
This commit is contained in:
Alyssa Rosenzweig 2025-02-04 11:11:22 -05:00 committed by Marge Bot
parent 41eabbadfa
commit e3bc6eafc8
3 changed files with 25 additions and 35 deletions

View file

@ -2337,9 +2337,7 @@ nir_gen_rect_vertices(nir_builder *b, nir_def *z, nir_def *w);
/* Emits a printf in the same way nir_lower_printf(). Each of the variadic
* argument is a pointer to a nir_def value.
*/
void nir_printf_fmt(nir_builder *b,
bool use_printf_base_identifier,
unsigned ptr_bit_size,
void nir_printf_fmt(nir_builder *b, unsigned ptr_bit_size,
const char *fmt, ...);
/* Call a serialized function. This is used internally by vtn_bindgen, it is not

View file

@ -206,21 +206,9 @@ nir_lower_printf_buffer(nir_shader *nir, uint64_t address, uint32_t size)
}
void
nir_printf_fmt(nir_builder *b,
bool use_printf_base_identifier,
unsigned ptr_bit_size,
const char *fmt, ...)
nir_printf_fmt(nir_builder *b, unsigned ptr_bit_size, const char *fmt, ...)
{
b->shader->printf_info_count++;
b->shader->printf_info = reralloc(b->shader,
b->shader->printf_info,
u_printf_info,
b->shader->printf_info_count);
u_printf_info *info =
&b->shader->printf_info[b->shader->printf_info_count - 1];
*info = (u_printf_info) {
u_printf_info info = {
.strings = ralloc_strdup(b->shader, fmt),
.string_size = strlen(fmt) + 1,
};
@ -253,11 +241,10 @@ nir_printf_fmt(nir_builder *b,
ASSERTED nir_def *def = va_arg(ap, nir_def*);
assert(def->bit_size / 8 == arg_size);
info->num_args++;
info->arg_sizes = reralloc(b->shader,
info->arg_sizes,
unsigned, info->num_args);
info->arg_sizes[info->num_args - 1] = arg_size;
info.num_args++;
info.arg_sizes = reralloc(b->shader, info.arg_sizes, unsigned,
info.num_args);
info.arg_sizes[info.num_args - 1] = arg_size;
args_size += arg_size;
}
@ -272,20 +259,13 @@ nir_printf_fmt(nir_builder *b,
.atomic_op = nir_atomic_op_iadd);
uint32_t total_size = sizeof(uint32_t); /* identifier */
for (unsigned a = 0; a < info->num_args; a++)
total_size += info->arg_sizes[a];
for (unsigned a = 0; a < info.num_args; a++)
total_size += info.arg_sizes[a];
nir_push_if(b, nir_ilt(b, nir_iadd_imm(b, buffer_offset, total_size),
nir_load_printf_buffer_size(b)));
{
/* Identifier */
nir_def *identifier =
use_printf_base_identifier ?
nir_iadd_imm(b,
nir_load_printf_base_identifier(b),
b->shader->printf_info_count) :
nir_imm_int(b, b->shader->printf_info_count);
nir_def *identifier = nir_imm_int(b, u_printf_hash(&info));
nir_def *store_addr =
nir_iadd(b, buffer_addr, nir_u2uN(b, buffer_offset, buffer_addr->bit_size));
nir_store_global(b, store_addr, 4, identifier, 0x1);
@ -293,15 +273,27 @@ nir_printf_fmt(nir_builder *b,
/* Arguments */
va_start(ap, fmt);
unsigned store_offset = sizeof(uint32_t);
for (unsigned a = 0; a < info->num_args; a++) {
for (unsigned a = 0; a < info.num_args; a++) {
nir_def *def = va_arg(ap, nir_def*);
nir_store_global(b, nir_iadd_imm(b, store_addr, store_offset),
4, def, 0x1);
store_offset += info->arg_sizes[a];
store_offset += info.arg_sizes[a];
}
va_end(ap);
}
nir_pop_if(b, NULL);
/* Add the format string to the printf singleton, registering the hash for
* the driver. This isn't actually correct, because the shader may be cached
* and reused in the future but the singleton will die along with the logical
* device. However, nir_printf_fmt is a debugging aid used in conjunction
* with directly modifying the Mesa code, there are never uses of
* nir_printf_fmt checked into the tree. Rebuilding Mesa invalidates the disk
* cache anyway, so this will more or less do what we want without requiring
* lots of extra plumbing to soften this edge case. And disabling the disk
* cache while debugging compiler issues is a good practice anyway.
*/
u_printf_singleton_add(&info, 1);
}

View file

@ -897,7 +897,7 @@ print_ubo_load(nir_builder *b,
return false;
b->cursor = nir_after_instr(&intrin->instr);
nir_printf_fmt(b, true, 64,
nir_printf_fmt(b, 64,
"uniform<= pos=%02.2fx%02.2f offset=0x%08x val=0x%08x\n",
nir_channel(b, nir_load_frag_coord(b), 0),
nir_channel(b, nir_load_frag_coord(b), 1),