On Tue, Oct 20, 2015 at 1:19 AM, Iago Toral <ito...@igalia.com> wrote:
> On Mon, 2015-10-19 at 23:11 -0700, Matt Turner wrote:
>> On Mon, Oct 19, 2015 at 9:09 PM, Matt Turner <matts...@gmail.com> wrote:
>> > We implement textureQueryLevels (which takes no arguments, save the
>> > sampler) using the resinfo message (which takes an argument of LOD).
>> > Without initializing it, we'd generate a MOV from the null register to
>> > load the LOD argument.
>> > ---
>> > src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> > b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> > index ea1e3e7..c942c80 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> > @@ -1579,7 +1579,7 @@ vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
>> > const glsl_type *coord_type = NULL;
>> > src_reg shadow_comparitor;
>> > src_reg offset_value;
>> > - src_reg lod, lod2;
>> > + src_reg lod(0.0f), lod2;
>> > src_reg sample_index;
>> > src_reg mcs;
>>
>> The fs backend handles this differently --
>>
>> if (op == ir_query_levels) {
>> /* textureQueryLevels() is implemented in terms of TXS so we need to
>> * pass a valid LOD argument.
>> */
>> assert(lod.file == BAD_FILE);
>> lod = fs_reg(0u);
>> }
>>
>> That kinda seems worse -- but it does indicate that I should probably
>> initialize lod with 0u instead, though it doesn't seem to matter...
>
>
> I don't care much either way, the good thing about the code in the fs
> backend is that it makes things more explicit. I think I kind of like
> that a bit better but works for me either way. Whatever we do, we should
> probably use the same solution in both backends though.
Agreed. I'll send a new patch that implements it using the fs way.
>> mov(8) g10<1>F [0F, 0F, 0F, 0F]VF
>> send(8) g3<1>UW g10<8,8,1>F
>> sampler resinfo SIMD8 Surface = 1 Sampler = 0 mlen 1 rlen 4
>>
>> Opinions?
>
> isn't LOD a float value by definition? Also, there are other parts of
> the FS code that initialize it to 0.0, like this (from
> lower_sampler_logical_send_gen7):
>
> if (bld.shader->stage != MESA_SHADER_FRAGMENT &&
> op == SHADER_OPCODE_TEX) {
> op = SHADER_OPCODE_TXL;
> lod = fs_reg(0.0f);
> }
Interesting. I wonder if we should be doing that at a higher level (in
GLSL IR, for instance).
> In any case, in that same function we do:
>
> for (unsigned i = 0; i < ARRAY_SIZE(sources); i++)
> sources[i] = bld.vgrf(BRW_REGISTER_TYPE_F);
>
> and then later we do:
>
> bld.MOV(sources[length], lod)
>
> so even if we initialize lod to 0u it is going to end up as a float,
> which explains the result you obtained, so I think float is probably
> what we want.
Yep, that explains it. Thanks for tracking that down.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
