nir: Drop nir_ssa_def::name and nir_register::name

We say that they're for debug only but we don't really have a good
policy around when to set them and when not to.  In particular,
nir_lower_system_values and nir_lower_vars_to_ssa which are the chief
producers of SSA values which might reasonably have a name do not bother
to set one.  We have some names set from things like BLORP and RADV's
meta shaders but AFAICT, they're setting a name more because it's there
than because they actually care.

Also, most things other than nir_clone and nir_serialize don't bother to
try and preserve them.  You can see in the diffstat of this commit
exactly what passes attempt to preserve names.  Notably missing from the
list is opt_algebraic which is the single largest source of SSA def
churn and it happily throws names away.

These observations lead me to question whether or not names are actually
useful at all or if they're just taking up space (8B per instruction)
and wasting CPU cycles (to ralloc_strdup on the off chance we do have
one).  I don't think I can think of a single time in recent history
where I've been debugging a shader issue and a SSA value name has been
there and been useful.  If anything, the few times they are there, they
just throw me off because they mess up the indentation in nir_print.

iris shader-db on my system gets runtime -2.07734% +/- 1.26933% (n=5)

Reviewed-by: Emma Anholt <emma@anholt.net>
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5439>
This commit is contained in:
Jason Ekstrand 2020-06-11 18:23:07 -05:00 committed by Marge Bot
parent a8173a78a3
commit 624e799cc3
18 changed files with 27 additions and 66 deletions

View file

@ -140,7 +140,6 @@ reg_create(void *mem_ctx, struct exec_list *list)
reg->num_components = 0;
reg->bit_size = 32;
reg->num_array_elems = 0;
reg->name = NULL;
exec_list_push_tail(list, &reg->node);
@ -621,7 +620,7 @@ nir_load_const_instr_create(nir_shader *shader, unsigned num_components,
rzalloc_size(shader, sizeof(*instr) + num_components * sizeof(*instr->value));
instr_init(&instr->instr, nir_instr_type_load_const);
nir_ssa_def_init(&instr->instr, &instr->def, num_components, bit_size, NULL);
nir_ssa_def_init(&instr->instr, &instr->def, num_components, bit_size);
return instr;
}
@ -769,7 +768,7 @@ nir_ssa_undef_instr_create(nir_shader *shader,
nir_ssa_undef_instr *instr = ralloc(shader, nir_ssa_undef_instr);
instr_init(&instr->instr, nir_instr_type_ssa_undef);
nir_ssa_def_init(&instr->instr, &instr->def, num_components, bit_size, NULL);
nir_ssa_def_init(&instr->instr, &instr->def, num_components, bit_size);
return instr;
}
@ -1457,9 +1456,8 @@ nir_instr_rewrite_dest(nir_instr *instr, nir_dest *dest, nir_dest new_dest)
void
nir_ssa_def_init(nir_instr *instr, nir_ssa_def *def,
unsigned num_components,
unsigned bit_size, const char *name)
unsigned bit_size)
{
def->name = ralloc_strdup(instr, name);
def->parent_instr = instr;
list_inithead(&def->uses);
list_inithead(&def->if_uses);
@ -1486,7 +1484,7 @@ nir_ssa_dest_init(nir_instr *instr, nir_dest *dest,
const char *name)
{
dest->is_ssa = true;
nir_ssa_def_init(instr, &dest->ssa, num_components, bit_size, name);
nir_ssa_def_init(instr, &dest->ssa, num_components, bit_size);
}
void

View file

@ -728,9 +728,6 @@ typedef struct nir_register {
/** generic register index. */
unsigned index;
/** only for debug purposes, can be NULL */
const char *name;
/** set of nir_srcs where this register is used (read from) */
struct list_head uses;
@ -806,9 +803,6 @@ nir_instr_is_last(const nir_instr *instr)
}
typedef struct nir_ssa_def {
/** for debugging only, can be NULL */
const char* name;
/** Instruction which produces this SSA value. */
nir_instr *parent_instr;
@ -4020,8 +4014,7 @@ void nir_ssa_dest_init(nir_instr *instr, nir_dest *dest,
unsigned num_components, unsigned bit_size,
const char *name);
void nir_ssa_def_init(nir_instr *instr, nir_ssa_def *def,
unsigned num_components, unsigned bit_size,
const char *name);
unsigned num_components, unsigned bit_size);
static inline void
nir_ssa_dest_init_for_type(nir_instr *instr, nir_dest *dest,
const struct glsl_type *type,

View file

@ -212,7 +212,6 @@ clone_register(clone_state *state, const nir_register *reg)
nreg->bit_size = reg->bit_size;
nreg->num_array_elems = reg->num_array_elems;
nreg->index = reg->index;
nreg->name = ralloc_strdup(nreg, reg->name);
/* reconstructing uses/defs/if_uses handled by nir_instr_insert() */
list_inithead(&nreg->uses);
@ -258,7 +257,7 @@ __clone_dst(clone_state *state, nir_instr *ninstr,
ndst->is_ssa = dst->is_ssa;
if (dst->is_ssa) {
nir_ssa_dest_init(ninstr, ndst, dst->ssa.num_components,
dst->ssa.bit_size, dst->ssa.name);
dst->ssa.bit_size, NULL);
if (likely(state->remap_table))
add_remap(state, &ndst->ssa, &dst->ssa);
} else {

View file

@ -740,7 +740,7 @@ rematerialize_deref_in_block(nir_deref_instr *deref,
nir_ssa_dest_init(&new_deref->instr, &new_deref->dest,
deref->dest.ssa.num_components,
deref->dest.ssa.bit_size,
deref->dest.ssa.name);
NULL);
nir_builder_instr_insert(b, &new_deref->instr);
return new_deref;

View file

@ -141,10 +141,7 @@ merge_set_dump(merge_set *set, FILE *fp)
for (int i = 0; i <= dom_idx; i++)
fprintf(fp, " ");
if (node->def->name)
fprintf(fp, "ssa_%d /* %s */\n", node->def->index, node->def->name);
else
fprintf(fp, "ssa_%d\n", node->def->index);
fprintf(fp, "ssa_%d\n", node->def->index);
dom[++dom_idx] = node->def;
}
@ -384,7 +381,7 @@ isolate_phi_nodes_block(nir_block *block, void *dead_ctx)
nir_parallel_copy_entry);
nir_ssa_dest_init(&pcopy->instr, &entry->dest,
phi->dest.ssa.num_components,
phi->dest.ssa.bit_size, src->src.ssa->name);
phi->dest.ssa.bit_size, NULL);
entry->dest.ssa.divergent = nir_src_is_divergent(src->src);
exec_list_push_tail(&pcopy->entries, &entry->node);
@ -399,7 +396,7 @@ isolate_phi_nodes_block(nir_block *block, void *dead_ctx)
nir_parallel_copy_entry);
nir_ssa_dest_init(&block_pcopy->instr, &entry->dest,
phi->dest.ssa.num_components, phi->dest.ssa.bit_size,
phi->dest.ssa.name);
NULL);
entry->dest.ssa.divergent = phi->dest.ssa.divergent;
exec_list_push_tail(&block_pcopy->entries, &entry->node);
@ -504,7 +501,6 @@ create_reg_for_ssa_def(nir_ssa_def *def, nir_function_impl *impl)
{
nir_register *reg = nir_local_reg_create(impl);
reg->name = def->name;
reg->num_components = def->num_components;
reg->bit_size = def->bit_size;
reg->num_array_elems = 0;
@ -775,7 +771,6 @@ resolve_parallel_copy(nir_parallel_copy_instr *pcopy,
*/
assert(num_vals < num_copies * 2);
nir_register *reg = nir_local_reg_create(state->builder.impl);
reg->name = "copy_temp";
reg->num_array_elems = 0;
if (values[b].is_ssa) {
reg->num_components = values[b].ssa->num_components;

View file

@ -1381,7 +1381,7 @@ build_explicit_io_load(nir_builder *b, nir_intrinsic_instr *intrin,
assert(intrin->dest.is_ssa);
load->num_components = num_components;
nir_ssa_dest_init(&load->instr, &load->dest, num_components,
bit_size, intrin->dest.ssa.name);
bit_size, NULL);
assert(bit_size % 8 == 0);
@ -1680,7 +1680,7 @@ build_explicit_io_atomic(nir_builder *b, nir_intrinsic_instr *intrin,
assert(intrin->dest.ssa.num_components == 1);
nir_ssa_dest_init(&atomic->instr, &atomic->dest,
1, intrin->dest.ssa.bit_size, intrin->dest.ssa.name);
1, intrin->dest.ssa.bit_size, NULL);
assert(atomic->dest.ssa.bit_size % 8 == 0);

View file

@ -96,7 +96,7 @@ rewrite_dest(nir_dest *dest, void *_state)
list_del(&dest->reg.def_link);
nir_ssa_dest_init(instr, dest, reg->num_components,
reg->bit_size, reg->name);
reg->bit_size, NULL);
nir_phi_builder_value_set_block_def(value, instr->block, &dest->ssa);
@ -179,7 +179,7 @@ rewrite_alu_instr(nir_alu_instr *alu, struct regs_to_ssa_state *state)
alu->dest.write_mask = (1 << num_components) - 1;
list_del(&alu->dest.dest.reg.def_link);
nir_ssa_dest_init(&alu->instr, &alu->dest.dest, num_components,
reg->bit_size, reg->name);
reg->bit_size, NULL);
nir_op vecN_op = nir_op_vec(reg->num_components);
@ -200,7 +200,7 @@ rewrite_alu_instr(nir_alu_instr *alu, struct regs_to_ssa_state *state)
}
nir_ssa_dest_init(&vec->instr, &vec->dest.dest, reg->num_components,
reg->bit_size, reg->name);
reg->bit_size, NULL);
nir_instr_insert(nir_after_instr(&alu->instr), &vec->instr);
nir_phi_builder_value_set_block_def(value, alu->instr.block,

View file

@ -1043,7 +1043,7 @@ clone_alu_and_replace_src_defs(nir_builder *b, const nir_alu_instr *alu,
nir_ssa_dest_init(&nalu->instr, &nalu->dest.dest,
alu->dest.dest.ssa.num_components,
alu->dest.dest.ssa.bit_size, alu->dest.dest.ssa.name);
alu->dest.dest.ssa.bit_size, NULL);
nalu->dest.saturate = alu->dest.saturate;
nalu->dest.write_mask = alu->dest.write_mask;

View file

@ -461,7 +461,7 @@ nir_opt_peephole_select_block(nir_block *block, nir_shader *shader,
nir_ssa_dest_init(&sel->instr, &sel->dest.dest,
phi->dest.ssa.num_components,
phi->dest.ssa.bit_size, phi->dest.ssa.name);
phi->dest.ssa.bit_size, NULL);
sel->dest.write_mask = (1 << phi->dest.ssa.num_components) - 1;
nir_ssa_def_rewrite_uses(&phi->dest.ssa,

View file

@ -82,8 +82,6 @@ static void
print_register(nir_register *reg, print_state *state)
{
FILE *fp = state->fp;
if (reg->name != NULL)
fprintf(fp, "/* %s */ ", reg->name);
fprintf(fp, "r%u", reg->index);
}
@ -107,8 +105,6 @@ static void
print_ssa_def(nir_ssa_def *def, print_state *state)
{
FILE *fp = state->fp;
if (def->name != NULL)
fprintf(fp, "/* %s */ ", def->name);
fprintf(fp, "%s %u ssa_%u", sizes[def->num_components], def->bit_size,
def->index);
}
@ -117,8 +113,6 @@ static void
print_ssa_use(nir_ssa_def *def, print_state *state)
{
FILE *fp = state->fp;
if (def->name != NULL)
fprintf(fp, "/* %s */ ", def->name);
fprintf(fp, "ssa_%u", def->index);
}

View file

@ -442,9 +442,6 @@ write_register(write_ctx *ctx, const nir_register *reg)
blob_write_uint32(ctx->blob, reg->bit_size);
blob_write_uint32(ctx->blob, reg->num_array_elems);
blob_write_uint32(ctx->blob, reg->index);
blob_write_uint32(ctx->blob, !ctx->strip && reg->name);
if (!ctx->strip && reg->name)
blob_write_string(ctx->blob, reg->name);
}
static nir_register *
@ -456,13 +453,6 @@ read_register(read_ctx *ctx)
reg->bit_size = blob_read_uint32(ctx->blob);
reg->num_array_elems = blob_read_uint32(ctx->blob);
reg->index = blob_read_uint32(ctx->blob);
bool has_name = blob_read_uint32(ctx->blob);
if (has_name) {
const char *name = blob_read_string(ctx->blob);
reg->name = ralloc_strdup(reg, name);
} else {
reg->name = NULL;
}
list_inithead(&reg->uses);
list_inithead(&reg->defs);
@ -573,9 +563,9 @@ union packed_dest {
uint8_t u8;
struct {
uint8_t is_ssa:1;
uint8_t has_name:1;
uint8_t num_components:3;
uint8_t bit_size:3;
uint8_t _pad:1;
} ssa;
struct {
uint8_t is_ssa:1;
@ -699,7 +689,6 @@ write_dest(write_ctx *ctx, const nir_dest *dst, union packed_instr header,
dest.ssa.is_ssa = dst->is_ssa;
if (dst->is_ssa) {
dest.ssa.has_name = !ctx->strip && dst->ssa.name;
dest.ssa.num_components =
encode_num_components_in_3bits(dst->ssa.num_components);
dest.ssa.bit_size = encode_bit_size_3bits(dst->ssa.bit_size);
@ -753,8 +742,6 @@ write_dest(write_ctx *ctx, const nir_dest *dst, union packed_instr header,
if (dst->is_ssa) {
write_add_object(ctx, &dst->ssa);
if (dest.ssa.has_name)
blob_write_string(ctx->blob, dst->ssa.name);
} else {
blob_write_uint32(ctx->blob, write_lookup_object(ctx, dst->reg.reg));
blob_write_uint32(ctx->blob, dst->reg.base_offset);
@ -777,8 +764,7 @@ read_dest(read_ctx *ctx, nir_dest *dst, nir_instr *instr,
num_components = blob_read_uint32(ctx->blob);
else
num_components = decode_num_components_in_3bits(dest.ssa.num_components);
char *name = dest.ssa.has_name ? blob_read_string(ctx->blob) : NULL;
nir_ssa_dest_init(instr, dst, num_components, bit_size, name);
nir_ssa_dest_init(instr, dst, num_components, bit_size, NULL);
read_add_object(ctx, &dst->ssa);
} else {
dst->reg.reg = read_object(ctx);

View file

@ -559,7 +559,7 @@ ir3_get_array(struct ir3_context *ctx, nir_register *reg)
if (arr->r == reg)
return arr;
}
ir3_context_error(ctx, "bogus reg: %s\n", reg->name);
ir3_context_error(ctx, "bogus reg: r%d\n", reg->index);
return NULL;
}

View file

@ -333,7 +333,7 @@ lower_ssbo_ubo_intrinsic(nir_builder *b, nir_intrinsic_instr *intrin)
nir_ssa_dest_init(&copy->instr, &copy->dest,
intrin->dest.ssa.num_components,
intrin->dest.ssa.bit_size,
intrin->dest.ssa.name);
NULL);
results[i] = &copy->dest.ssa;
}

View file

@ -495,7 +495,7 @@ crocus_setup_uniforms(const struct brw_compiler *compiler,
nir_ssa_dest_init(&load_ubo->instr, &load_ubo->dest,
intrin->dest.ssa.num_components,
intrin->dest.ssa.bit_size,
intrin->dest.ssa.name);
NULL);
nir_builder_instr_insert(&b, &load_ubo->instr);
nir_ssa_def_rewrite_uses(&intrin->dest.ssa,

View file

@ -48,8 +48,7 @@ lima_nir_duplicate_intrinsic(nir_builder *b, nir_intrinsic_instr *itr,
dupl->src[0].reg = itr->src[0].reg;
nir_ssa_dest_init(&dupl->instr, &dupl->dest,
dupl->num_components, itr->dest.ssa.bit_size,
itr->dest.ssa.name);
dupl->num_components, itr->dest.ssa.bit_size, NULL);
dupl->instr.pass_flags = 1;
nir_builder_instr_insert(b, &dupl->instr);
@ -82,8 +81,7 @@ lima_nir_duplicate_intrinsic(nir_builder *b, nir_intrinsic_instr *itr,
dupl->src[0].reg = itr->src[0].reg;
nir_ssa_dest_init(&dupl->instr, &dupl->dest,
dupl->num_components, itr->dest.ssa.bit_size,
itr->dest.ssa.name);
dupl->num_components, itr->dest.ssa.bit_size, NULL);
dupl->instr.pass_flags = 1;
nir_builder_instr_insert(b, &dupl->instr);

View file

@ -65,8 +65,7 @@ dup_mem_intrinsic(nir_builder *b, nir_intrinsic_instr *intrin,
if (info->has_dest) {
assert(intrin->dest.is_ssa);
nir_ssa_dest_init(&dup->instr, &dup->dest,
num_components, bit_size,
intrin->dest.ssa.name);
num_components, bit_size, NULL);
} else {
nir_intrinsic_set_write_mask(dup, (1 << num_components) - 1);
}

View file

@ -250,8 +250,7 @@ brw_nir_opt_peephole_ffma_block(nir_builder *b, nir_block *block)
nir_ssa_dest_init(&ffma->instr, &ffma->dest.dest,
add->dest.dest.ssa.num_components,
bit_size,
add->dest.dest.ssa.name);
bit_size, NULL);
nir_ssa_def_rewrite_uses(&add->dest.dest.ssa,
&ffma->dest.dest.ssa);

View file

@ -774,7 +774,7 @@ lower_shared_atomic(nir_builder *b, nir_intrinsic_instr *intr,
atomic->src[2] = nir_src_for_ssa(intr->src[2].ssa);
}
atomic->num_components = 0;
nir_ssa_dest_init(&atomic->instr, &atomic->dest, 1, 32, intr->dest.ssa.name);
nir_ssa_dest_init(&atomic->instr, &atomic->dest, 1, 32, NULL);
nir_builder_instr_insert(b, &atomic->instr);
nir_ssa_def_rewrite_uses(&intr->dest.ssa, &atomic->dest.ssa);