On Mar 23, 2016 12:32 AM, "Samuel Iglesias Gonsálvez" <<a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> wrote: > > On 21/03/16 23:40, Jason Ekstrand wrote: > > On Mon, Mar 21, 2016 at 3:39 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> > > wrote: > > > >> > >> > >> On Mon, Mar 21, 2016 at 5:05 AM, Samuel Iglesias Gonsálvez < > >> <a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> wrote: > >> > >>> From: Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> > >>> > >>> 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]. That seems reasonable as long as we don't break anything along the way. > 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 I don't really care too much much one way or the other. I think my mild preference would be to keep them unsized. (A texture instruction returning an fp16 value can happen on some hardware). My point is that if we do keep those hunks, they need to be squashed together with a tgsi_to_nir patch and corresponding back-end fixes. > And do the corresponding modifications to the driver. > > Thanks! > > Sam > > [0] > <a href="https://github.com/Igalia/mesa/commit/16c477ab84227d8bb9ffcba07091aef9a5e9f61e">https://github.com/Igalia/mesa/commit/16c477ab84227d8bb9ffcba07091aef9a5e9f61e</a> > > >> 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 > >>> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > >>> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > >>> > >> > >> > >