[Mesa-dev] [PATCH 2/2] i965/cfg: Ignore non-CF instructions in unreachable blocks.

[Matt Turner]

On Thu, Aug 18, 2016 at 10:21 AM, Matt Turner <mattst88 at gmail.com> wrote:
> On Thu, Aug 18, 2016 at 3:43 AM, Iago Toral <itoral at igalia.com> wrote:
>> On Wed, 2016-08-17 at 11:54 -0700, Matt Turner wrote:
>>> The basic block following a control flow structure like an infinite
>>> loop
>>> will be unreachable. Ignore any non-control-flow instructions in it
>>> since they can have no effect on the program.
>>
>> If the block is unreachable control-flow instructions inside the block
>> are also irrelevant, is there any reason why you don't skip CF
>> instructions too?
>
> I think that could lead to further problems. For instance, if we had
>
> START B47
>   do
>   break
> END B47
> START B48
>   while
> END B48
>
> B48 would be unreachable, but I think emitting a "do" instruction
> without a "while" might cause problems in the generator
> (brw_find_loop_end, brw_find_next_block_end).
>
>>> Avoids a segmentation fault in cfg_t::intersect(a, b) when called on
>>> an
>>> unreachable block. By avoiding ever putting code in an unreachable
>>> block, we never attempt to optimize code in an unreachable block.
>>
>> Can't the problem persist if the unreachable block has any control-flow
>> instructions?
>
> I don't think so. The problem involved finding a constant
> (fs_visitor::opt_combine_constants) that was in both a reachable and
> an unreachable block. When a constant is in two different blocks, the
> code finds the common ancestor of both blocks (cfg_t::intersect) and
> places the constant in it. If one of the blocks is unreachable, it
> will not have an immediate dominator, leading to cfg_t::intersect
> segfaulting. Since control flow instructions do not take regular
> sources (and not immediates), they should pose no problem.
>
> The background is that Jason noticed a segfault when enabling GCM/GVN
> in NIR in a single piglit test
> (tests/glslparsertest/shaders/CorrectFull.frag). The test is a silly
> parser test that contains an infinite loop. After enabling GVN, a
> constant was used in both a reachable and unreachable block, leading
> to the segfault.
>
> My fix is to not put instructions into unreachable basic blocks. :)

I talked with Curro. He's right that this isn't sufficient (one could
have an unreachable block with many unreachable descendants full of
instructions).

I'll drop this patch.