[Mesa-dev] [PATCH 19/29] nir: fix up bit sizes for undefined alu sources

[Jason Ekstrand]

On Mar 22, 2016 8:18 AM, "Samuel Iglesias Gonsálvez" <siglesias at igalia.com>
wrote:
>
>
>
> On 21/03/16 23:54, Jason Ekstrand wrote:
> > On Mon, Mar 21, 2016 at 5:05 AM, Samuel Iglesias Gonsálvez <
> > siglesias at igalia.com> wrote:
> >
> >> From: Iago Toral Quiroga <itoral at igalia.com>
> >>
> >> 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
> >> mesa-dev at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >>
> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160322/b24ca56d/attachment.html>