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

[Richard Biener <rguenther at suse dot de>]

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).

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);
 


> > --- a/gcc/omp-expand.c
> > +++ b/gcc/omp-expand.c
> > @@ -5439,6 +5439,13 @@ expand_oacc_for (struct omp_region *region, struct omp_for_data *fd)
> >  
> >  	  split->flags ^= EDGE_FALLTHRU | EDGE_TRUE_VALUE;
> >  
> > +	  /* Add a dummy exit for the tiled block when cont_bb is missing.  */
> > +	  if (cont_bb == NULL)
> > +	    {
> > +	      edge e = make_edge (body_bb, exit_bb, EDGE_FALSE_VALUE);
> > +	      e->probability = profile_probability::even ();
> > +	    }
> 
> I miss here updating of split->probability, if you make e even (the other edge
> needs to have probability of 100%-the probability, i.e. even as well.
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)