On Mar 22, 2016 8:18 AM, "Samuel Iglesias Gonsálvez" <<a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> wrote: > > > > On 21/03/16 23:54, Jason Ekstrand wrote: > > On Mon, Mar 21, 2016 at 5:05 AM, Samuel Iglesias Gonsálvez < > > <a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> wrote: > > > >> From: Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> > >> > >> Undefined sources in alu operations don't have a valid bit size because > >> they are uninitialized. Simply ignoring undefined sources for bit size > >> validation is not enough since drivers can check and operate with the > >> bit-size and that can lead to issues later on. Instead, fix undefined > >> sources to always have a compatible bit size. > >> > > > > I'm not sure what I think about this. I think I'd rather have undefs > > simply have the right bitsize. > > > > With undefined sources you cannot get the bitsize from themselves > because it is not initialized. Doesn't patch 3 and the discussion on it imply that undefs should have valid sizes? > In that case, we pick the bit size from > the ALU opcode's input definition. If it is unsized, then we use the > destination size. I dont think pulling the implicit size from the source size is ever correct. If you have an explicitly sized source that means it doesn't affect and isn't affected by the implicit size. > I think this is the right bitsize... or am I missing something? > > Sam > > > > >> v2 (Sam): > >> - Use helper to get type size from nir_alu_type. > >> --- > >> src/compiler/nir/nir_validate.c | 10 ++++++++++ > >> 1 file changed, 10 insertions(+) > >> > >> diff --git a/src/compiler/nir/nir_validate.c > >> b/src/compiler/nir/nir_validate.c > >> index 9f18d1c..645c15a 100644 > >> --- a/src/compiler/nir/nir_validate.c > >> +++ b/src/compiler/nir/nir_validate.c > >> @@ -180,9 +180,11 @@ validate_alu_src(nir_alu_instr *instr, unsigned > >> index, validate_state *state) > >> > >> unsigned num_components; > >> unsigned src_bit_size; > >> + bool is_undef = false; > >> if (src->src.is_ssa) { > >> src_bit_size = src->src.ssa->bit_size; > >> num_components = src->src.ssa->num_components; > >> + is_undef = src->src.ssa->parent_instr->type == > >> nir_instr_type_ssa_undef; > >> } else { > >> src_bit_size = src->src.reg.reg->bit_size; > >> if (src->src.reg.reg->is_packed) > >> @@ -205,12 +207,20 @@ validate_alu_src(nir_alu_instr *instr, unsigned > >> index, validate_state *state) > >> > >> if (nir_alu_type_get_type_size(src_type)) { > >> /* This source has an explicit bit size */ > >> + if (is_undef) { > >> + src_bit_size = nir_alu_type_get_type_size(src_type); > >> + src->src.ssa->bit_size = src_bit_size; > >> + } > >> assert(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; > >> + if (is_undef) { > >> + src_bit_size = dest_bit_size; > >> + src->src.ssa->bit_size = dest_bit_size; > >> + } > >> assert(dest_bit_size == src_bit_size); > >> } > >> } > >> -- > >> 2.5.0 > >> > >> _______________________________________________ > >> mesa-dev mailing list > >> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > >> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > >> > >