[Mesa-dev] [PATCH] nir: allow sat on all float destination types

[Jason Ekstrand]

On May 31, 2016 11:33 PM, "Ilia Mirkin" <imirkin at alum.mit.edu> wrote:
>
> On Wed, Jun 1, 2016 at 2:14 AM, Jason Ekstrand <jason at jlekstrand.net>
wrote:
> >
> > On May 31, 2016 2:50 PM, "Ilia Mirkin" <imirkin at alum.mit.edu> wrote:
> >>
> >> With the introduction of fp64 and fp16 to nir, there are now a bunch of
> >> float types running around. A F1 2015 shader ends up with an i2f.sat
> >> operation, which has a nir_type_float32 destination. Allow sat on all
> >> the float destination types.
> >>
> >> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> >> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
> >> ---
> >>  src/compiler/nir/nir_validate.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/src/compiler/nir/nir_validate.c
> >> b/src/compiler/nir/nir_validate.c
> >> index 35bb162..8404e11 100644
> >> --- a/src/compiler/nir/nir_validate.c
> >> +++ b/src/compiler/nir/nir_validate.c
> >> @@ -331,7 +331,7 @@ validate_alu_dest(nir_alu_instr *instr,
validate_state
> >> *state)
> >>      * destinations of type float
> >>      */
> >>     nir_alu_instr *alu = nir_instr_as_alu(state->instr);
> >> -   validate_assert(state, nir_op_infos[alu->op].output_type ==
> >> nir_type_float ||
> >> +   validate_assert(state, nir_op_infos[alu->op].output_type &
> >> nir_type_float ||
> >
> > We have a nir_alu_type_get_base_type helper function which would
probably be
> > more appropriate than the &.  With that changed,
> >
> > Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
>
> Thanks! I'm having a really hard time formatting it in a way that
> doesn't look ridiculous -- something like this?

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

> diff --git a/src/compiler/nir/nir_validate.c
b/src/compiler/nir/nir_validate.c
> index 35bb162..e5f5b8a 100644
> --- a/src/compiler/nir/nir_validate.c
> +++ b/src/compiler/nir/nir_validate.c
> @@ -331,7 +331,9 @@ validate_alu_dest(nir_alu_instr *instr,
> validate_state *state)
>      * destinations of type float
>      */
>     nir_alu_instr *alu = nir_instr_as_alu(state->instr);
> -   validate_assert(state, nir_op_infos[alu->op].output_type ==
> nir_type_float ||
> +   validate_assert(state,
> +          (nir_alu_type_get_base_type(nir_op_infos[alu->op].output_type)
==
> +           nir_type_float) ||
>            !dest->saturate);
>
>     unsigned bit_size = dest->dest.is_ssa ? dest->dest.ssa.bit_size
>
> Or should I pull out the type calculation like it is done below for
> instr->op (I assume that state->instr != instr?)
>
> >
> >>            !dest->saturate);
> >>
> >>     unsigned bit_size = dest->dest.is_ssa ? dest->dest.ssa.bit_size
> >> --
> >> 2.7.3
> >>
> >> _______________________________________________
> >> 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/20160601/eeca6ad4/attachment-0001.html>