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

[Aldy Hernandez <aldyh@redhat.com>]

On 9/9/21 2:52 PM, Michael Matz wrote:
> Hello,
> 
> On Thu, 9 Sep 2021, Aldy Hernandez wrote:
> 
>> The ldist-22 regression is interesting though:
>>
>> void foo ()
>> {
>>    int i;
>>
>>    <bb 2> :
>>    goto <bb 6>; [INV]
>>
>>    <bb 3> :
>>    a[i_1] = 0;
>>    if (i_1 > 100)
>>      goto <bb 4>; [INV]
>>    else
>>      goto <bb 5>; [INV]
>>
>>    <bb 4> :
>>    b[i_1] = i_1;
>>
>>    <bb 5> :
>>    i_8 = i_1 + 1;
>>
>>    <bb 6> :
>>    # i_1 = PHI <0(2), i_8(5)>
>>    if (i_1 <= 1023)
>>      goto <bb 3>; [INV]
>>    else
>>      goto <bb 7>; [INV]
> 
> 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:

Checking profitability of path (backwards):  bb:6 (2 insns) bb:5 (latch)

> 
>> Would it be crazy to suggest that we disable threading through latches
>> altogether,
> 
> I think it wouldn't be crazy, but we can do a bit better as suggested
> above (only reject empty latches, and reject it only for the threaders
> coming before the loop optims).

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
> @@ -600,6 +600,7 @@ back_threader_profitability::profitable_path_p (const vec<basic_block> &m_path,
>    loop_p loop = m_path[0]->loop_father;
>    bool path_crosses_loops = false;
>    bool threaded_through_latch = false;
> +  bool latch_within_path = false;
>    bool multiway_branch_in_path = false;
>    bool threaded_multiway_branch = false;
>    bool contains_hot_bb = false;
> @@ -725,7 +726,13 @@ back_threader_profitability::profitable_path_p (const vec<basic_block> &m_path,
>  	 the last entry in the array when determining if we thread
>  	 through the loop latch.  */
>        if (loop->latch == bb)
> -	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.

Aldy