<p dir="ltr"><br>
On May 31, 2016 11:33 PM, "Ilia Mirkin" <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>> wrote:<br>
><br>
> On Wed, Jun 1, 2016 at 2:14 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> ><br>
> > On May 31, 2016 2:50 PM, "Ilia Mirkin" <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>> wrote:<br>
> >><br>
> >> With the introduction of fp64 and fp16 to nir, there are now a bunch of<br>
> >> float types running around. A F1 2015 shader ends up with an i2f.sat<br>
> >> operation, which has a nir_type_float32 destination. Allow sat on all<br>
> >> the float destination types.<br>
> >><br>
> >> Signed-off-by: Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>><br>
> >> Cc: "12.0" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>><br>
> >> ---<br>
> >>  src/compiler/nir/nir_validate.c | 2 +-<br>
> >>  1 file changed, 1 insertion(+), 1 deletion(-)<br>
> >><br>
> >> diff --git a/src/compiler/nir/nir_validate.c<br>
> >> b/src/compiler/nir/nir_validate.c<br>
> >> index 35bb162..8404e11 100644<br>
> >> --- a/src/compiler/nir/nir_validate.c<br>
> >> +++ b/src/compiler/nir/nir_validate.c<br>
> >> @@ -331,7 +331,7 @@ validate_alu_dest(nir_alu_instr *instr, validate_state<br>
> >> *state)<br>
> >>      * destinations of type float<br>
> >>      */<br>
> >>     nir_alu_instr *alu = nir_instr_as_alu(state->instr);<br>
> >> -   validate_assert(state, nir_op_infos[alu->op].output_type ==<br>
> >> nir_type_float ||<br>
> >> +   validate_assert(state, nir_op_infos[alu->op].output_type &<br>
> >> nir_type_float ||<br>
> ><br>
> > We have a nir_alu_type_get_base_type helper function which would probably be<br>
> > more appropriate than the &.  With that changed,<br>
> ><br>
> > Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br>
><br>
> Thanks! I'm having a really hard time formatting it in a way that<br>
> doesn't look ridiculous -- something like this?</p>
<p dir="ltr">I knew that was going to be the hard part of this patch the moment I saw how long the function name was...  Meh?  I don't think there is a "good" solution.  If the output type is used elsewhere, it might be worth pulling it out.  Otherwise what you have is fine.  You could also let it run past 80 characters.  I won't tell anyone if you don't. :-p</p>
<p dir="ltr">> diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c<br>
> index 35bb162..e5f5b8a 100644<br>
> --- a/src/compiler/nir/nir_validate.c<br>
> +++ b/src/compiler/nir/nir_validate.c<br>
> @@ -331,7 +331,9 @@ validate_alu_dest(nir_alu_instr *instr,<br>
> validate_state *state)<br>
>      * destinations of type float<br>
>      */<br>
>     nir_alu_instr *alu = nir_instr_as_alu(state->instr);<br>
> -   validate_assert(state, nir_op_infos[alu->op].output_type ==<br>
> nir_type_float ||<br>
> +   validate_assert(state,<br>
> +          (nir_alu_type_get_base_type(nir_op_infos[alu->op].output_type) ==<br>
> +           nir_type_float) ||<br>
>            !dest->saturate);<br>
><br>
>     unsigned bit_size = dest->dest.is_ssa ? dest->dest.ssa.bit_size<br>
><br>
> Or should I pull out the type calculation like it is done below for<br>
> instr->op (I assume that state->instr != instr?)<br>
><br>
> ><br>
> >>            !dest->saturate);<br>
> >><br>
> >>     unsigned bit_size = dest->dest.is_ssa ? dest->dest.ssa.bit_size<br>
> >> --<br>
> >> 2.7.3<br>
> >><br>
> >> _______________________________________________<br>
> >> mesa-dev mailing list<br>
> >> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> >> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</p>