On Fri, Apr 24, 2015 at 5:15 PM, Neil Roberts <n...@linux.intel.com> wrote:
> Matt Turner <matts...@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 <n...@linux.intel.com> wrote:
> Matt Turner <matts...@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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
