[Mesa-dev] [PATCH 1/5/2] i965/fs: Bail out of opt_sampler_eot() if last instruction isn't EOT.

[Matt Turner]

On Wed, Aug 17, 2016 at 3:30 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> Matt Turner <mattst88 at gmail.com> writes:
>
>> ... instead of assert failing. Can only happen when the program has an
>> unconditional infinite loop.
>
> I'm curious how the framebuffer write gets eliminated, I don't think DCE
> is smart enough currently to find out that the FB write is unreachable?

Sorry, I typo'd the subject. It should have been PATCH 1.5/2. 2/2
makes a change to cfg_t() to not move non-control flow instructions
into the bblock_t instruction list when the bblock_t has no parents.
In the case an infinite loop immediately before the final block
containing the fbwrite, it leaves the (already completely broken)
program without an fbwrite.

>
>> ---
>> Sigh.
>>
>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index d1ac80a..08e9b6c 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -2511,8 +2511,9 @@ fs_visitor::opt_sampler_eot()
>>     /* Look for a texturing instruction immediately before the final FB_WRITE. */
>>     bblock_t *block = cfg->blocks[cfg->num_blocks - 1];
>>     fs_inst *fb_write = (fs_inst *)block->end();
>> -   assert(fb_write->eot);
>> -   assert(fb_write->opcode == FS_OPCODE_FB_WRITE_LOGICAL);
>> +   if (unlikely(!fb_write->eot) ||
>> +       unlikely(fb_write->opcode != FS_OPCODE_FB_WRITE_LOGICAL))
>> +      return false;
>>
>
> Maybe we want something in between the two?  Like:
>
> |   if (unlikely(fb_write->opcode != FS_OPCODE_FB_WRITE_LOGICAL))
> |      return false;
> |
> |   assert(fb_write->eot);
>
> My thinking is that *if* the final instruction of the shader is a
> framebuffer write it should have already been marked EOT during
> translation.

That's true. I'll make that change. Thanks.