[Mesa-dev] [PATCH] i965/fs: Strip trailing contant zeroes in sample messages

[Matt Turner]

On Fri, Apr 24, 2015 at 5:15 PM, Neil Roberts <neil at linux.intel.com> wrote:
> Matt Turner <mattst88 at gmail.com> writes:
>
>>> +   foreach_block_and_inst(block, fs_inst, inst, cfg) {
>>> +      if ((inst->opcode == SHADER_OPCODE_TEX ||
>>> +           inst->opcode == SHADER_OPCODE_TXF) &&
>>> +          !inst->shadow_compare) {
>>> +         fs_inst *load_payload = (fs_inst *) inst->prev;
>>> +
>>> +         if (load_payload->is_head_sentinel() ||
>>> +             load_payload->opcode != SHADER_OPCODE_LOAD_PAYLOAD)
>>> +            continue;
>>
>> We can't guarantee that the load_payload isn't used by another texture
>> later in the program, and since you need to change the texture
>> operation's mlen, I think you need to check that the load_payload
>> isn't used after this texture operation.
>>
>> To do that, (1) add an ip variable and initialize it to -1, (2) add
>> ip++ as the first statement in the foreach_block_and_inst loop, (3)
>> add some code to this check similar to in
>> brw_fs_saturate_propagation.cpp using this->live_intervals.
>
> Hrm, I'm not modifying the LOAD_PAYLOAD instruction, only the
> SHADRE_OPCODE_TEX/TXF instruction. If there is a later instruction that
> is using the LOAD_PAYLOAD, won't that end up making it's own dependency
> on the MOV instructions so they won't get removed?

On Fri, Apr 24, 2015 at 5:15 PM, Neil Roberts <neil at linux.intel.com> wrote:
> Matt Turner <mattst88 at gmail.com> writes:
>
>>> +   foreach_block_and_inst(block, fs_inst, inst, cfg) {
>>> +      if ((inst->opcode == SHADER_OPCODE_TEX ||
>>> +           inst->opcode == SHADER_OPCODE_TXF) &&
>>> +          !inst->shadow_compare) {
>>> +         fs_inst *load_payload = (fs_inst *) inst->prev;
>>> +
>>> +         if (load_payload->is_head_sentinel() ||
>>> +             load_payload->opcode != SHADER_OPCODE_LOAD_PAYLOAD)
>>> +            continue;
>>
>> We can't guarantee that the load_payload isn't used by another texture
>> later in the program, and since you need to change the texture
>> operation's mlen, I think you need to check that the load_payload
>> isn't used after this texture operation.
>>
>> To do that, (1) add an ip variable and initialize it to -1, (2) add
>> ip++ as the first statement in the foreach_block_and_inst loop, (3)
>> add some code to this check similar to in
>> brw_fs_saturate_propagation.cpp using this->live_intervals.
>
> Hrm, I'm not modifying the LOAD_PAYLOAD instruction, only the
> SHADRE_OPCODE_TEX/TXF instruction. If there is a later instruction that
> is using the LOAD_PAYLOAD, won't that end up making it's own dependency
> on the MOV instructions so they won't get removed?

Oh, you're right. I misread what the patch was doing. Indeed, I think
this should work fine.

>>> +   OPT(opt_zero_samples);
>>
>> I think you're probably right that this can be done after the
>> optimization loop. I guess it's possible that we might trim a texture
>> payload down and it'll then be the same as an existing payload and we
>> can then CSE them. I'd be interested to see if putting it inside the
>> optimization loop improves anything.
>
> Hrm, it might be worth trying. However, as I mentioned above, I'm not
> modifying the LOAD_PAYLOAD instruction so it probably won't hit the
> example you mentioned.

Right, okay. I don't mind if you want to try that later.