[Mesa-dev] [PATCH 07/29] nir/lower_tex: fix get_zero_or_one() to use sized types

[Samuel Iglesias Gonsálvez]

On 21/03/16 23:40, Jason Ekstrand wrote:
> On Mon, Mar 21, 2016 at 3:39 PM, Jason Ekstrand <jason at jlekstrand.net>
> 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>
>>>
>>> v2 (Sam):
>>> - Use helper to get type size from nir_alu_type enum.
>>> ---
>>>  src/compiler/nir/nir_lower_tex.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/compiler/nir/nir_lower_tex.c
>>> b/src/compiler/nir/nir_lower_tex.c
>>> index 4999603..a58385a 100644
>>> --- a/src/compiler/nir/nir_lower_tex.c
>>> +++ b/src/compiler/nir/nir_lower_tex.c
>>> @@ -226,7 +226,8 @@ get_zero_or_one(nir_builder *b, nir_alu_type type,
>>> uint8_t swizzle_val)
>>>        v.u32[0] = v.u32[1] = v.u32[2] = v.u32[3] = 0;
>>>     } else {
>>>        assert(swizzle_val == 5);
>>> -      if (type == nir_type_float)
>>> +      assert(nir_alu_type_get_type_size(type) < 64);
>>> +      if (type == nir_type_float32)
>>>
>>
>> This seems dangerious.  It switches a check from checking for float to
>> checking for float32.  At the particular point in the git history where
>> this patch lands, one of these checks is broken.  I'm guessing this
>> actually breaks things that get fixed later when we switch glsl_to_nir to
>> giving us sized types.
>>

Right, Connor modified glsl_to_nir to add sized types to the destination
type. Actually, these patches should come after it, we reordered them
when preparing this series for submission.

>> If we're going to change tex instructions to use sized types, we need to
>> do it all in one big patch and it will have to touch all three drivers
>> again.
>>
> 
> One more thought:  Tex instructions already have a bit size provided by the
> destination so I don't see a need for having it be sized at all.  At the
> end of the day it doesn't matter since we either don't have a size or we do
> have a size and it's required to match.
> 
> 

Yeah, right. Currently brw_glsl_base_type_for_nir_type() and
brw_type_for_nir_type() manage unsized types but our idea was to support
only sized types in the driver at the end of the fp64 changes. See this
patch [0].

If we are not going to change tex instructions to use sized types, then
we just need to get rid of patch [0] because otherwise it would break
tex instructions emission.

What do you think?

If we agree with keeping unsized types for tex instructions, I will
remove these patches from the series:

   nir/lower_tex: fix get_zero_or_one() to use sized types
   nir/lower_tex: fix get_texture_size() to use sized types
   program/nir: include bit-size information

Delete the respective ir_visitor::visit(ir_texture *ir) hunk from:

   nir/glsl_to_nir: support doubles

And do the corresponding modifications to the driver.

Thanks!

Sam

[0]
https://github.com/Igalia/mesa/commit/16c477ab84227d8bb9ffcba07091aef9a5e9f61e

>>           v.f32[0] = v.f32[1] = v.f32[2] = v.f32[3] = 1.0;
>>>        else
>>>           v.u32[0] = v.u32[1] = v.u32[2] = v.u32[3] = 1;
>>> --
>>> 2.5.0
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>
>>
>