Re: [PATCH] Remove verify_no_unreachable_blocks

[Tom de Vries <tdevries at suse dot de>]

On 08/23/2018 02:06 PM, Richard Biener wrote:
> On Thu, 23 Aug 2018, Tom de Vries wrote:
> 
>> On 08/23/2018 01:18 PM, Richard Biener wrote:
>>> This removes verify_no_unreachable_blocks and implements checking
>>> for unreachable blocks in inverted_post_order_compute by simply
>>> looking if we reach a block without predecessors that is not the
>>> entry block.
>>>
>>
>> I think that doesn't detect unreachable cyles: say you have blocks A and
>> B, and A -> B -> A.
> 
> Ah, true.  Bah, I guess I can use some other flag in my pass.
> 
>>> This solves a problem I ran into when (ab-)using BB_REACHABLE
>>> in a pass and I got comparison failues because of -fchecking vs.
>>> -fno-checking.  It also should speed up checking builds.
>>>
>>> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>>>
>>> Tom, does this make sense?
>>>
>>
>> The intent of the check I added was to verify the assumption stated in
>> the comment. I don't know how serious it is if the assumption is violated.
> 
> I think if you have reverse-not-reachable blocks (infinite loops w/o
> fake exit edges) that are not reachable from entry it will ICE
> or loop infintely.
> 

Hmm, looking at the code a bit more, there's an infinite loop detection
mechanism, and I think the case I described above, would run into this
assert:
...
      if (has_unvisited_bb && stack.is_empty ())
        {
          /* No blocks are reachable from EXIT at all.

             Find a dead-end from the ENTRY, and restart the
             iteration. */
          basic_block be
            = dfs_find_deadend (ENTRY_BLOCK_PTR_FOR_FN (cfun));
          gcc_assert (be != NULL);
          bitmap_set_bit (visited, be->index);
          stack.quick_push (ei_start (be->preds));
        }
...

So perhaps the code already verifies that there are no unreachable
blocks, and the comment needs to be updated to:
...
   This function requires that all blocks in the CFG are reachable

   from the ENTRY (but not necessarily from EXIT).  Otherwise, it ICEs.
...

Thanks,
- Tom