[PR44503] Fixes "control flow in the middle of basic block" with -fprefetch-loop-arrays

Hi,

Attached is the patch to fix bug 44503: "control flow in the middle of basic block" with -fprefetch-loop-arrays.

The problem is that a non-local label (for setjmp) exists in the function. When we insert a _builtin_prefetch
call, this prefetch is considered potentially changing the control flow. We think this is, in general, a loop 
construction problem. When the current function has non local labels, there are no natural loops in the 
function. 

The patch passed bootstrapping and gcc regression tests on amd-linux64 systems.

Is it ok for the trunk?

Thanks,

Changpeng

> Hi,
> 
> Attached is the patch to fix bug 44503: "control flow in the middle of basic block" with -fprefetch-loop-arrays.
> 
> The problem is that a non-local label (for setjmp) exists in the function. When we insert a _builtin_prefetch
> call, this prefetch is considered potentially changing the control flow. We think this is, in general, a loop 
> construction problem. When the current function has non local labels, there are no natural loops in the 
> function. 

I would rather say it is CFG problem - for known builtins, like
__bultiln_prefetch we certainly don't need to produce nonlocal goto edges.

In fact this might be perhaps bundled in my "leaf" attribute patch I proposed
some time ago.

Honza
> 
> The patch passed bootstrapping and gcc regression tests on amd-linux64 systems.
> 
> Is it ok for the trunk?
> 
> Thanks,
> 
> Changpeng
Content-Description: 0003-Do-not-form-natural-loops-when-non-local-labels-exis.patch
> From 91dd3df467e121426131cf477a77eb07d6062438 Mon Sep 17 00:00:00 2001
> From: Changpeng Fang <chfang@houghton.(none)>
> Date: Mon, 14 Jun 2010 16:46:06 -0700
> Subject: [PATCH 3/3] Do not form natural loops when non-local labels exist in the current function
> 
> 	*cfgloop.c (flow_loops_find): When the current function has non local
> 	labels, there are no natural loops in the function.
> ---
>  gcc/cfgloop.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/gcc/cfgloop.c b/gcc/cfgloop.c
> index 858e75b..d2ba445 100644
> --- a/gcc/cfgloop.c
> +++ b/gcc/cfgloop.c
> @@ -394,6 +394,18 @@ flow_loops_find (struct loops *loops)
>        return 1;
>      }
>  
> + /* When the current function has non local labels, there are no
> +     natural loops in the function.  */
> +  if (cfun->has_nonlocal_label)
> +    {
> +      init_loops_structure (loops, 1);
> +
> +      FOR_EACH_BB (bb)
> +	bb->loop_father = loops->tree_root;
> +
> +      return 1;
> +    }
> +
>    dfs_order = NULL;
>    rc_order = NULL;
>  
> -- 
> 1.6.3.3
>

Hi,
>
> >Attached is the patch to fix bug 44503: "control flow in the middle of basic block" with -fprefetch-loop-arrays.
>
> >The problem is that a non-local label (for setjmp) exists in the function. When we insert a _builtin_prefetch
> >call, this prefetch is considered potentially changing the control flow. We think this is, in general, a loop
> >construction problem. When the current function has non local labels, there are no natural loops in the
> >function.

>I would rather say it is CFG problem - for known builtins, like
>__bultiln_prefetch we certainly don't need to produce nonlocal goto edges.

You are right in this case. But in general, a function call may indirectly goto
the non-local labels. I didn't see any logic in gcc that handle this for loop 
construction.

>In fact this might be perhaps bundled in my "leaf" attribute patch I proposed
>some time ago.
But the problem occurs in the current gcc source.

Thanks,

Changpeng





Honza
>
> The patch passed bootstrapping and gcc regression tests on amd-linux64 systems.
>
> Is it ok for the trunk?
>
> Thanks,
>
> Changpeng
Content-Description: 0003-Do-not-form-natural-loops-when-non-local-labels-exis.patch
> From 91dd3df467e121426131cf477a77eb07d6062438 Mon Sep 17 00:00:00 2001
> From: Changpeng Fang <chfang@houghton.(none)>
> Date: Mon, 14 Jun 2010 16:46:06 -0700
> Subject: [PATCH 3/3] Do not form natural loops when non-local labels exist in the current function
>
>       *cfgloop.c (flow_loops_find): When the current function has non local
>       labels, there are no natural loops in the function.
> ---
>  gcc/cfgloop.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/gcc/cfgloop.c b/gcc/cfgloop.c
> index 858e75b..d2ba445 100644
> --- a/gcc/cfgloop.c
> +++ b/gcc/cfgloop.c
> @@ -394,6 +394,18 @@ flow_loops_find (struct loops *loops)
>        return 1;
>      }
>
> + /* When the current function has non local labels, there are no
> +     natural loops in the function.  */
> +  if (cfun->has_nonlocal_label)
> +    {
> +      init_loops_structure (loops, 1);
> +
> +      FOR_EACH_BB (bb)
> +     bb->loop_father = loops->tree_root;
> +
> +      return 1;
> +    }
> +
>    dfs_order = NULL;
>    rc_order = NULL;
>
> --
> 1.6.3.3
>

Hi,

> Attached is the patch to fix bug 44503: "control flow in the middle of basic block" with -fprefetch-loop-arrays.
> 
> The problem is that a non-local label (for setjmp) exists in the function. When we insert a _builtin_prefetch
> call, this prefetch is considered potentially changing the control flow. We think this is, in general, a loop 
> construction problem. When the current function has non local labels, there are no natural loops in the 
> function. 

this does not make much sense to me; why should there be no natural loops?  Edges in CFG (conservatively)
correctly describe the control flow, so the loops may be detected as usual.  The correct fix is to
ensure that is_ctrl_altering_stmt does not return true for_builtin_prefetch,

Zdenek

Hi,

> >I would rather say it is CFG problem - for known builtins, like
> >__bultiln_prefetch we certainly don't need to produce nonlocal goto edges.
> 
> You are right in this case. But in general, a function call may indirectly goto
> the non-local labels. I didn't see any logic in gcc that handle this for loop 
> construction.

no special logic is needed; possible jumps due to non-local labels are decribed by
CFG,

Zdenek

-fprefetch-loop-arrays.
>
> The problem is that a non-local label (for setjmp) exists in the function. When we insert a _builtin_prefetch
> call, this prefetch is considered potentially changing the control flow. We think this is, in general, a loop
> construction problem. When the current function has non local labels, there are no natural loops in the
> function.

>I would rather say it is CFG problem - for known builtins, like
>__bultiln_prefetch we certainly don't need to produce nonlocal goto edges.

>In fact this might be perhaps bundled in my "leaf" attribute patch I proposed
>some time ago.
 
Hi, Honza:

Would you please give a link to your leaf attribute patch ?

Thanks,

Changpeng

> -fprefetch-loop-arrays.
> >
> > The problem is that a non-local label (for setjmp) exists in the function. When we insert a _builtin_prefetch
> > call, this prefetch is considered potentially changing the control flow. We think this is, in general, a loop
> > construction problem. When the current function has non local labels, there are no natural loops in the
> > function.
> 
> >I would rather say it is CFG problem - for known builtins, like
> >__bultiln_prefetch we certainly don't need to produce nonlocal goto edges.
> 
> >In fact this might be perhaps bundled in my "leaf" attribute patch I proposed
> >some time ago.
>  
> Hi, Honza:
> 
> Would you please give a link to your leaf attribute patch ?

http://gcc.gnu.org/ml/gcc-patches/2010-06/msg01329.html

It allows to tag builtins to be "well behaved" that is not calling functions within current unit
and returning normally (or not at all).  I think non-local goto can be defined as not normal that
would allow cfg builder to not make edges on any leaf functions.

External function calls can do nonlocal goto back only via callback as far as I know.

Honza
> 
> Thanks,
> 
> Changpeng