[PATCH] Mark irreducible regions in the backwards threader

Aldy Hernandez aldyh@redhat.com
Tue Jun 15 09:33:39 GMT 2021



On 6/15/21 10:27 AM, Richard Biener wrote:
> On Tue, 15 Jun 2021, Richard Biener wrote:
> 
>> On Tue, 15 Jun 2021, Aldy Hernandez wrote:
>>
>>>
>>>
>>> On 6/14/21 4:04 PM, Richard Biener wrote:
>>>> The same issue that arises in PR100934 can happen in the backward
>>>> threader now (Aldy?) - we eventually run into mark_irreducible_loops
>>>> for non-FSM thread paths checking for paths crossing irreducible
>>>> region boundaries.
>>>
>>> I haven't been following the above PR, so I may be missing something, but it
>>> seems like you would need to twiddle all users of the path registry, not just
>>> the backwards threader.
>>
>> Yes, VRP already uses LOOPS_NORMAL, I fixed DOM with the patch for
>> PR100934.  That leaves the backwards threader, so after this change
>> all consumers should be fixed.
>>
>> The question was more like if we'd ever run into the code in
>> mark_threaded_blocks that reads
>>
>>    /* Look for jump threading paths which cross multiple loop headers.
>>
>>       The code to thread through loop headers will change the CFG in ways
>>       that invalidate the cached loop iteration information.  So we must
>>       detect that case and wipe the cached information.  */
>>    EXECUTE_IF_SET_IN_BITMAP (tmp, 0, i, bi)
>> ...
>>
>> we're definitely executing mark_threaded_blocks but we've executed
>> and pruned all FSM threading paths before.
>>
>> I suppose I should simply place the assert there and see what happens
>> when I don't fixup the backwards threader ...
> 
> diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
> index a86302be18e..1155b3b9a8f 100644
> --- a/gcc/tree-ssa-threadupdate.c
> +++ b/gcc/tree-ssa-threadupdate.c
> @@ -2122,6 +2122,7 @@ jump_thread_path_registry::mark_threaded_blocks
> (bitmap threaded_blocks)
>          {
>            if (e->aux)
>              {
> +             gcc_assert (loops_state_satisfies_p
> (LOOPS_HAVE_MARKED_IRREDUCIBLE_REGIONS));
>                vec<jump_thread_edge *> *path = THREAD_PATH (e);
>   
>                for (unsigned int i = 0, crossed_headers = 0;
> 
> passes bootstrap and regtest, so I suppose the backwards threader
> doesn't need the initialization right now.
> 
> I'll push this instead.

Thanks.  I'll keep an eye out with my proposed changes.

Aldy



More information about the Gcc-patches mailing list