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

[Matt Turner]

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. :)

See the wip/nir-gvn branch of my tree.

Thanks for reviewing!