[Mesa-dev] [PATCH 1/4] st/glsl_to_tgsi: simplify translate_tex_offset

[Nicolai Hähnle]

On 12.10.2016 20:12, Ilia Mirkin wrote:
> Patches 1-3 are
>
> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>

Thanks.

> Not comfortable enough with the vertex arrays code to review the last
> one. I remember Dave had semi-good reasons for the if (!array) stuff
> though.

As far as I understood the code, the if (!array) was used to jump over 
the placeholders for the secondary slots of dvec3/dvec4. That should be 
covered by the fact that init_velement_lower is used to advance the attr 
index appropriately.

If there's a reason that I missed, that would be good to know :)

Cheers,
Nicolai

>
> On Wed, Oct 12, 2016 at 1:50 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>
>> This fixes a bug with offsets from uniforms which seems to have only been
>> noticed as a crash in piglit's
>> arb_gpu_shader5/compiler/builtin-functions/fs-gatherOffset-uniform-offset.frag
>> on radeonsi.
>>
>> Cc: mesa-stable at lists.freedesktop.org
>> ---
>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 64 +++++++-----------------------
>>  1 file changed, 14 insertions(+), 50 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index f721506..33c1f87 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -5661,74 +5661,38 @@ translate_src(struct st_translate *t, const st_src_reg *src_reg)
>>     if (src_reg->reladdr != NULL) {
>>        assert(src_reg->file != PROGRAM_TEMPORARY);
>>        src = ureg_src_indirect(src, ureg_src(t->address[0]));
>>     }
>>
>>     return src;
>>  }
>>
>>  static struct tgsi_texture_offset
>>  translate_tex_offset(struct st_translate *t,
>> -                     const st_src_reg *in_offset, int idx)
>> +                     const st_src_reg *in_offset)
>>  {
>>     struct tgsi_texture_offset offset;
>> -   struct ureg_src imm_src;
>> -   struct ureg_dst dst;
>> -   int array;
>> +   struct ureg_src src = translate_src(t, in_offset);
>>
>> -   switch (in_offset->file) {
>> -   case PROGRAM_IMMEDIATE:
>> -      assert(in_offset->index >= 0 && in_offset->index < t->num_immediates);
>> -      imm_src = t->immediates[in_offset->index];
>> -
>> -      offset.File = imm_src.File;
>> -      offset.Index = imm_src.Index;
>> -      offset.SwizzleX = imm_src.SwizzleX;
>> -      offset.SwizzleY = imm_src.SwizzleY;
>> -      offset.SwizzleZ = imm_src.SwizzleZ;
>> -      offset.Padding = 0;
>> -      break;
>> -   case PROGRAM_INPUT:
>> -      imm_src = t->inputs[t->inputMapping[in_offset->index]];
>> -      offset.File = imm_src.File;
>> -      offset.Index = imm_src.Index;
>> -      offset.SwizzleX = GET_SWZ(in_offset->swizzle, 0);
>> -      offset.SwizzleY = GET_SWZ(in_offset->swizzle, 1);
>> -      offset.SwizzleZ = GET_SWZ(in_offset->swizzle, 2);
>> -      offset.Padding = 0;
>> -      break;
>> -   case PROGRAM_TEMPORARY:
>> -      imm_src = ureg_src(t->temps[in_offset->index]);
>> -      offset.File = imm_src.File;
>> -      offset.Index = imm_src.Index;
>> -      offset.SwizzleX = GET_SWZ(in_offset->swizzle, 0);
>> -      offset.SwizzleY = GET_SWZ(in_offset->swizzle, 1);
>> -      offset.SwizzleZ = GET_SWZ(in_offset->swizzle, 2);
>> -      offset.Padding = 0;
>> -      break;
>> -   case PROGRAM_ARRAY:
>> -      array = in_offset->index >> 16;
>> +   offset.File = src.File;
>> +   offset.Index = src.Index;
>> +   offset.SwizzleX = src.SwizzleX;
>> +   offset.SwizzleY = src.SwizzleY;
>> +   offset.SwizzleZ = src.SwizzleZ;
>> +   offset.Padding = 0;
>>
>> -      assert(array >= 0);
>> -      assert(array < (int)t->num_temp_arrays);
>> +   assert(!src.Indirect);
>> +   assert(!src.DimIndirect);
>> +   assert(!src.Dimension);
>> +   assert(!src.Absolute); /* those shouldn't be used with integers anyway */
>> +   assert(!src.Negate);
>>
>> -      dst = t->arrays[array];
>> -      offset.File = dst.File;
>> -      offset.Index = dst.Index + (in_offset->index & 0xFFFF) - 0x8000;
>> -      offset.SwizzleX = GET_SWZ(in_offset->swizzle, 0);
>> -      offset.SwizzleY = GET_SWZ(in_offset->swizzle, 1);
>> -      offset.SwizzleZ = GET_SWZ(in_offset->swizzle, 2);
>> -      offset.Padding = 0;
>> -      break;
>> -   default:
>> -      break;
>> -   }
>>     return offset;
>>  }
>>
>>  static void
>>  compile_tgsi_instruction(struct st_translate *t,
>>                           const glsl_to_tgsi_instruction *inst)
>>  {
>>     struct ureg_program *ureg = t->ureg;
>>     int i;
>>     struct ureg_dst dst[2];
>> @@ -5778,21 +5742,21 @@ compile_tgsi_instruction(struct st_translate *t,
>>     case TGSI_OPCODE_TXL2:
>>     case TGSI_OPCODE_TG4:
>>     case TGSI_OPCODE_LODQ:
>>        src[num_src] = t->samplers[inst->sampler.index];
>>        assert(src[num_src].File != TGSI_FILE_NULL);
>>        if (inst->sampler.reladdr)
>>           src[num_src] =
>>              ureg_src_indirect(src[num_src], ureg_src(t->address[2]));
>>        num_src++;
>>        for (i = 0; i < (int)inst->tex_offset_num_offset; i++) {
>> -         texoffsets[i] = translate_tex_offset(t, &inst->tex_offsets[i], i);
>> +         texoffsets[i] = translate_tex_offset(t, &inst->tex_offsets[i]);
>>        }
>>        tex_target = st_translate_texture_target(inst->tex_target, inst->tex_shadow);
>>
>>        ureg_tex_insn(ureg,
>>                      inst->op,
>>                      dst, num_dst,
>>                      tex_target,
>>                      texoffsets, inst->tex_offset_num_offset,
>>                      src, num_src);
>>        return;
>> --
>> 2.7.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev