Re: [PATCH] Handle empty infinite loops in OpenACC for PR84955

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Apr 11, 2018 at 9:30 PM, Cesar Philippidis
<cesar@codesourcery.com> wrote:
> On 04/09/2018 04:31 AM, Richard Biener wrote:
>> On Fri, 6 Apr 2018, Jakub Jelinek wrote:
>>
>>> On Fri, Apr 06, 2018 at 06:48:52AM -0700, Cesar Philippidis wrote:
>>>> 2018-04-06  Cesar Philippidis  <cesar@codesourcery.com>
>>>>
>>>>     PR middle-end/84955
>>>>
>>>>     gcc/
>>>>     * cfgloop.c (flow_loops_find): Add assert.
>>>>     * omp-expand.c (expand_oacc_for): Add dummy false branch for
>>>>         tiled basic blocks without omp continue statements.
>>>>     * tree-cfg.c (execute_fixup_cfg): Handle calls to internal
>>>>         functions like regular functions.
>>>>
>>>>     libgomp/
>>>>     * testsuite/libgomp.oacc-c-c++-common/pr84955.c: New test.
>>>>     * testsuite/libgomp.oacc-fortran/pr84955.f90: New test.
>>>
>>> I'd like to defer the cfgloop.c and tree-cfg.c changes to Richard, just want to
>>> mention that:
>>>
>>>> --- a/gcc/tree-cfg.c
>>>> +++ b/gcc/tree-cfg.c
>>>> @@ -9586,10 +9586,7 @@ execute_fixup_cfg (void)
>>>>        for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);)
>>>>     {
>>>>       gimple *stmt = gsi_stmt (gsi);
>>>> -     tree decl = is_gimple_call (stmt)
>>>> -                 ? gimple_call_fndecl (stmt)
>>>> -                 : NULL;
>>>> -     if (decl)
>>>> +     if (is_gimple_call (stmt))
>>>
>>> This change doesn't affect just internal functions, but also all indirect
>>> calls through function pointers with const, pure or noreturn attributes.
>>
>> I think the change is desirable nevertheless.  The question is if we
>> want to do it at this point in time.
>>
>> The description of the problem sounds more like LTO writing writing out
>> loops without previously fixing up state.  So sth like the following
>> which I'd prefer at this stage (the above hunk is ok for stage1 then).
>
> OK, I'll save that hunk for stage 1.
>
>> Index: gcc/lto-streamer-out.c
>> ===================================================================
>> --- gcc/lto-streamer-out.c      (revision 259227)
>> +++ gcc/lto-streamer-out.c      (working copy)
>> @@ -2084,6 +2151,9 @@ output_function (struct cgraph_node *nod
>>    /* Set current_function_decl and cfun.  */
>>    push_cfun (fn);
>>
>> +  /* Fixup loops if required to match discovery done in the reader.  */
>> +  loop_optimizer_init (AVOID_CFG_MODIFICATIONS);
>> +
>>    /* Make string 0 be a NULL string.  */
>>    streamer_write_char_stream (ob->string_stream, 0);
>>
>> @@ -2176,12 +2246,13 @@ output_function (struct cgraph_node *nod
>>        streamer_write_record_start (ob, LTO_null);
>>
>>        output_cfg (ob, fn);
>> -
>> -      pop_cfun ();
>>     }
>>    else
>>      streamer_write_uhwi (ob, 0);
>>
>> +  loop_optimizer_finalize ();
>> +  pop_cfun ();
>> +
>>    /* Create a section to hold the pickled output of this function.   */
>>    produce_asm (ob, function);
>
> That worked. Is this patch OK for trunk, GCC 6 and GCC 7?

Ok if you remove the cfgloop.c hunk.  There's no point in an assert
of sth being non-NULL when the immediately following stmt will
dereference it.  You get an ICE anyway.

Thanks,
Richard.

> Thanks,
> Cesar
>