This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch,gomp-4_0-branch] acc nested function support


Hi Cesar!

On Fri, 18 Jul 2014 14:07:00 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
> This patch enables acc constructs to be used inside nested functions.

Thanks!

> I
> doubt nested functions will be used much in c, but some of the openacc
> fortran tutorials I've seen online make use of internal subroutines in
> fortran. Those internal subroutines are implemented as nested functions.

> Does this look OK to commit to gomp-4_0-branch?

I think we first need to resolve the following issue:

> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -576,22 +576,6 @@ struct GTY((tag("GSS_OMP_PARALLEL_LAYOUT")))
>    tree data_arg;
>  };
>  
> -/* GIMPLE_OACC_KERNELS */
> -struct GTY((tag("GSS_OMP_PARALLEL_LAYOUT")))
> -  gimple_statement_oacc_kernels : public gimple_statement_omp_parallel_layout
> -{
> -    /* No extra fields; adds invariant:
> -         stmt->code == GIMPLE_OACC_KERNELS.  */
> -};
> -
> -/* GIMPLE_OACC_PARALLEL */
> -struct GTY((tag("GSS_OMP_PARALLEL_LAYOUT")))
> -  gimple_statement_oacc_parallel : public gimple_statement_omp_parallel_layout
> -{
> -    /* No extra fields; adds invariant:
> -         stmt->code == GIMPLE_OACC_PARALLEL.  */
> -};
> -
>  /* GIMPLE_OMP_PARALLEL or GIMPLE_TASK */
>  struct GTY((tag("GSS_OMP_PARALLEL_LAYOUT")))
>    gimple_statement_omp_taskreg : public gimple_statement_omp_parallel_layout
> @@ -617,6 +601,22 @@ struct GTY((tag("GSS_OMP_PARALLEL_LAYOUT")))
>           stmt->code == GIMPLE_OMP_TARGET.  */
>  };
>  
> +/* GIMPLE_OACC_KERNELS */
> +struct GTY((tag("GSS_OMP_PARALLEL_LAYOUT")))
> +  gimple_statement_oacc_kernels : public gimple_statement_omp_taskreg
> +{
> +    /* No extra fields; adds invariant:
> +         stmt->code == GIMPLE_OACC_KERNELS.  */
> +};
> +
> +/* GIMPLE_OACC_PARALLEL */
> +struct GTY((tag("GSS_OMP_PARALLEL_LAYOUT")))
> +  gimple_statement_oacc_parallel : public gimple_statement_omp_taskreg
> +{
> +    /* No extra fields; adds invariant:
> +         stmt->code == GIMPLE_OACC_PARALLEL.  */
> +};
> +
>  /* GIMPLE_OMP_TASK */
>  
>  struct GTY((tag("GSS_OMP_TASK")))
> @@ -927,7 +927,8 @@ template <>
>  inline bool
>  is_a_helper <gimple_statement_omp_taskreg *>::test (gimple gs)
>  {
> -  return gs->code == GIMPLE_OMP_PARALLEL || gs->code == GIMPLE_OMP_TASK;
> +  return gs->code == GIMPLE_OMP_PARALLEL || gs->code == GIMPLE_OMP_TASK
> +    || gs->code == GIMPLE_OACC_PARALLEL || gs->code == GIMPLE_OACC_KERNELS;
>  }
>  
>  template <>
> @@ -1135,7 +1136,8 @@ template <>
>  inline bool
>  is_a_helper <const gimple_statement_omp_taskreg *>::test (const_gimple gs)
>  {
> -  return gs->code == GIMPLE_OMP_PARALLEL || gs->code == GIMPLE_OMP_TASK;
> +  return gs->code == GIMPLE_OMP_PARALLEL || gs->code == GIMPLE_OMP_TASK
> +    || gs->code == GIMPLE_OACC_PARALLEL || gs->code == GIMPLE_OACC_KERNELS;
>  }

Doing it this way has been "disapproved" before, and I raised the issue
in
<http://news.gmane.org/find-root.php?message_id=%3C87egxcjkmc.fsf%40kepler.schwinge.homeip.net%3E>.
If that makes sense to you, please continue this discussion until I'm
back next week.

> --- a/gcc/tree-nested.c
> +++ b/gcc/tree-nested.c
> @@ -1322,7 +1320,22 @@ convert_nonlocal_reference_stmt (gimple_stmt_iterator *gsi, bool *handled_ops_p,
>  
>      case GIMPLE_OACC_KERNELS:
>      case GIMPLE_OACC_PARALLEL:
> -      gcc_unreachable ();
> +      save_suppress = info->suppress_expansion;
> +      convert_nonlocal_omp_clauses (gimple_omp_taskreg_clauses_ptr (stmt),
> +				    wi);
> +      save_local_var_chain = info->new_local_var_chain;
> +      info->new_local_var_chain = NULL;
> +
> +      walk_body (convert_nonlocal_reference_stmt, convert_nonlocal_reference_op,
> +	         info, gimple_omp_body_ptr (stmt));
> +
> +      if (info->new_local_var_chain)
> +	declare_vars (info->new_local_var_chain,
> +	              gimple_seq_first_stmt (gimple_omp_body (stmt)),
> +		      false);
> +      info->new_local_var_chain = save_local_var_chain;
> +      info->suppress_expansion = save_suppress;
> +      break;
>  
>      case GIMPLE_OMP_PARALLEL:
>      case GIMPLE_OMP_TASK:

Wouldn't we rather group the GIMPLE_OACC_* with GIMPLE_OMP_TARGET
(handling all these together)?  (Either way, please avoid the code
duplication for GIMPLE_OACC_KERNELS/GIMPLE_OACC_PARALLEL even if grouping
with GIMPLE_OMP_PARALLEL/GIMPLE_OMP_TASK; I think the latter's existing
code can basically be re-used as is?


GrÃÃe,
 Thomas

Attachment: pgpgNsOV4un4c.pgp
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]