More aggressive threading causing loop-interchange-9.c regression

[Aldy Hernandez]

On 9/9/21 4:44 PM, Michael Matz wrote:
> Hello,
> 
> On Thu, 9 Sep 2021, Aldy Hernandez wrote:
> 
>>> Here there's no simple latch block to start with (the backedge comes
>>> directly out of the loop exit block).  So my suggested improvement
>>> (testing if the latch was empty and only then reject the thread), would
>>> solve this.
>>
>> Well, there's the thing.  Loop discovery is marking BB5 as the latch, so
>> it's not getting threaded:
> 
> Yes, it's a latch, but not an empty one.  So the thread would make it just
> even more non-empty, which might not be a problem anymore.  So amending my
> patch somewhere with a strategic
> 
>    && empty_block_p (latch)
> 
> and only then rejecting the thread should make this testcase work again.

Sweet.

> 
> (There's still a catch, though: if this non-empty latch, which is also the
> exit test block, is threaded through and is followed by actual code, then
> that code will be inserted onto the back edge, not into the latch block
> before the exit test, and so also create a (new) non-empty latch.  That
> might or might not create problems downstreams, but not as many as
> transformaing an empty into a non-empty latch would do; but it could
> create for instance a second back edge (not in this testcase) and
> suchlike)
> 
>> BTW, I'm not sure your check for the non-last position makes a difference:
>>
>>> diff --git a/gcc/tree-ssa-threadbackward.c b/gcc/tree-ssa-threadbackward.c
>>> index 449232c7715..528a753b886 100644
>>> --- a/gcc/tree-ssa-threadbackward.c
>>> +++ b/gcc/tree-ssa-threadbackward.c
>>> -	threaded_through_latch = true;
>>> +	{
>>> +	  threaded_through_latch = true;
>>> +	  if (j != 0)
>>> +	    latch_within_path = true;
>>> +	  if (dump_file && (dump_flags & TDF_DETAILS))
>>> +	    fprintf (dump_file, " (latch)");
>>> +	}
>>>       }
>>
>> If the last position being considered is a simple latch, it only has a simple
>> outgoing jump.  There's no need to thread that.  You need a block with >= 2
>> succ edges to thread anything.
> 
> So, you are saying that any candidate thread path wouldn't have the latch
> in the last position if it were just an empty forwarder?  I simply wasn't
> sure about that, so was conservative and only wanted to reject things I
> knew where positively bad (namely code in the path following the latch
> that is in danger of being moved into the latch).

Correct.  In the backwards threader, we don't start the party if the 
last block has < 2 succs:

   FOR_EACH_BB_FN (bb, fun)
     {
       if (EDGE_COUNT (bb->succs) > 1)
	threader.maybe_thread_block (bb);
     }

Thanks.
Aldy