From a136884139c42a9ab96c659cbfe44149b1bbffdb Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Wed, 8 Mar 2017 10:32:40 -0800 Subject: [PATCH] nir/validate: Rework ALU bit-size rule validation The original bit-size validation wasn't capable of properly dealing with instructions with variable bit sizes. An attempt was made to handle it by looking at source and destinations but, because the validation was done in validate_alu_(src|dest), it didn't really have the needed information. The new validation code is much more straightforward and should be more correct. Reviewed-by: Eric Anholt --- src/compiler/nir/nir_validate.c | 65 +++++++++++++++++---------------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c index 16efcb2356c..cdbe6a6dced 100644 --- a/src/compiler/nir/nir_validate.c +++ b/src/compiler/nir/nir_validate.c @@ -227,12 +227,9 @@ validate_alu_src(nir_alu_instr *instr, unsigned index, validate_state *state) nir_alu_src *src = &instr->src[index]; unsigned num_components; - unsigned src_bit_size; if (src->src.is_ssa) { - src_bit_size = src->src.ssa->bit_size; num_components = src->src.ssa->num_components; } else { - src_bit_size = src->src.reg.reg->bit_size; if (src->src.reg.reg->is_packed) num_components = 4; /* can't check anything */ else @@ -245,24 +242,6 @@ validate_alu_src(nir_alu_instr *instr, unsigned index, validate_state *state) validate_assert(state, src->swizzle[i] < num_components); } - nir_alu_type src_type = nir_op_infos[instr->op].input_types[index]; - - /* 8-bit float isn't a thing */ - if (nir_alu_type_get_base_type(src_type) == nir_type_float) - validate_assert(state, src_bit_size == 16 || src_bit_size == 32 || src_bit_size == 64); - - if (nir_alu_type_get_type_size(src_type)) { - /* This source has an explicit bit size */ - validate_assert(state, nir_alu_type_get_type_size(src_type) == src_bit_size); - } else { - if (!nir_alu_type_get_type_size(nir_op_infos[instr->op].output_type)) { - unsigned dest_bit_size = - instr->dest.dest.is_ssa ? instr->dest.dest.ssa.bit_size - : instr->dest.dest.reg.reg->bit_size; - validate_assert(state, dest_bit_size == src_bit_size); - } - } - validate_src(&src->src, state, 0, 0); } @@ -369,17 +348,6 @@ validate_alu_dest(nir_alu_instr *instr, validate_state *state) nir_type_float) || !dest->saturate); - unsigned bit_size = dest->dest.is_ssa ? dest->dest.ssa.bit_size - : dest->dest.reg.reg->bit_size; - nir_alu_type type = nir_op_infos[instr->op].output_type; - - /* 8-bit float isn't a thing */ - if (nir_alu_type_get_base_type(type) == nir_type_float) - validate_assert(state, bit_size == 16 || bit_size == 32 || bit_size == 64); - - validate_assert(state, nir_alu_type_get_type_size(type) == 0 || - nir_alu_type_get_type_size(type) == bit_size); - validate_dest(&dest->dest, state, 0, 0); } @@ -388,10 +356,43 @@ validate_alu_instr(nir_alu_instr *instr, validate_state *state) { validate_assert(state, instr->op < nir_num_opcodes); + unsigned instr_bit_size = 0; for (unsigned i = 0; i < nir_op_infos[instr->op].num_inputs; i++) { + nir_alu_type src_type = nir_op_infos[instr->op].input_types[i]; + unsigned src_bit_size = nir_src_bit_size(instr->src[i].src); + if (nir_alu_type_get_type_size(src_type)) { + validate_assert(state, src_bit_size == nir_alu_type_get_type_size(src_type)); + } else if (instr_bit_size) { + validate_assert(state, src_bit_size == instr_bit_size); + } else { + instr_bit_size = src_bit_size; + } + + if (nir_alu_type_get_base_type(src_type) == nir_type_float) { + /* 8-bit float isn't a thing */ + validate_assert(state, src_bit_size == 16 || src_bit_size == 32 || + src_bit_size == 64); + } + validate_alu_src(instr, i, state); } + nir_alu_type dest_type = nir_op_infos[instr->op].output_type; + unsigned dest_bit_size = nir_dest_bit_size(instr->dest.dest); + if (nir_alu_type_get_type_size(dest_type)) { + validate_assert(state, dest_bit_size == nir_alu_type_get_type_size(dest_type)); + } else if (instr_bit_size) { + validate_assert(state, dest_bit_size == instr_bit_size); + } else { + /* The only unsized thing is the destination so it's vacuously valid */ + } + + if (nir_alu_type_get_base_type(dest_type) == nir_type_float) { + /* 8-bit float isn't a thing */ + validate_assert(state, dest_bit_size == 16 || dest_bit_size == 32 || + dest_bit_size == 64); + } + validate_alu_dest(instr, state); } -- 2.30.2