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

[Samuel Iglesias Gonsálvez]

On 22/03/16 17:37, Jason Ekstrand wrote:
> 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?
> 

Oh right. I will discard this patch as undefs already have valid sizes.

Thanks,

Sam

>> 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
>>>>
>>>
>