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

[Ilia Mirkin]

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?

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