[Mesa-dev] [PATCH] st/mesa: kill instruction if writemask=0 in eliminate_dead_code_advanced()

[Brian Paul]

I tested with a shader that generates an ARL instruction and found 
inst->dst is:

{file = PROGRAM_ADDRESS, index = 0, writemask = 1, cond_mask = 8, type 
= 2, reladdr = 0x0}

in the code in question.  I don't think there's any way that the dest 
register's writemask could be set to zero.

Other instructions that don't use the dst register (like 
OPCODE_BGNLOOP, OPCODE_IF, etc) use undef_dst to initialize the 
instructions dst register.

BTW, the initialization of undef_dst seems incorrect:

static st_dst_reg undef_dst = st_dst_reg(PROGRAM_UNDEFINED, 
SWIZZLE_NOOP, GLSL_TYPE_ERROR);

I think SWIZZLE_NOOP should be WRITEMASK_XYZW since the parameter is a 
writemask, not a swizzle.

-Brian



On 10/09/2011 01:22 PM, Marek Olšák wrote:
> Correct.
>
> The thing was: even though we have an addressing register in TGSI
> (e.g. tgsi_full_dst|src_register::Indirect), everybody except nv50 and
> partially svga doesn't use it now. They expect the indirect register
> file is always TGSI_FILE_ADDRESS and the index is 0, making the
> destination of ARL irrelevant. This is probably a bug in those drivers
> and that's what made me think that ARL doesn't need a destination
> register. Sorry for the noise.
>
> Marek
>
> On Sun, Oct 9, 2011 at 8:51 PM, Bryan Cain<bryancain3 at gmail.com>  wrote:
>> What does it do if there's no destination register?  In any case, I
>> don't think glsl_to_tgsi emits any ARLs of that form, so it shouldn't be
>> a problem.
>>
>> Bryan
>>
>> On 10/07/2011 01:06 PM, Marek Olšák wrote:
>>> I think ARL is allowed to have no destination register, right? In that
>>> case, there should be a special case not to eliminate ARLs.
>>>
>>> Marek
>>>
>>> On Fri, Oct 7, 2011 at 5:40 PM, Brian Paul<brian.e.paul at gmail.com>  wrote:
>>>> From: Brian Paul<brianp at vmware.com>
>>>>
>>>> This fixes a bug where we'd wind up emitting an invalid instruction like
>>>> MOVE R[0]., R[1];  - note the empty/zero writemask.  If we don't write to
>>>> any dest register channels, cull the instruction.
>>>> ---
>>>>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp |    8 +++++++-
>>>>   1 files changed, 7 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>>> index d8ef8a3..44b1149 100644
>>>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>>> @@ -3776,8 +3776,14 @@ glsl_to_tgsi_visitor::eliminate_dead_code_advanced(void)
>>>>           iter.remove();
>>>>           delete inst;
>>>>           removed++;
>>>> -      } else
>>>> +      } else {
>>>>           inst->dst.writemask&= ~(inst->dead_mask);
>>>> +         if (inst->dst.writemask == 0) {
>>>> +            iter.remove();
>>>> +            delete inst;
>>>> +            removed++;
>>>> +         }
>>>> +      }
>>>>     }
>>>>
>>>>     ralloc_free(write_level);
>>>> --
>>>> 1.7.3.4
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev