[omp] Move NE_EXPR handling to omp_adjust_for_condition

[Martin Jambor]

Ping please, the issue is now PR 89302.

Thanks,

Martin

On Fri, Feb 01 2019, Martin Jambor wrote:
> Hi,
>
> even after the two previous HSA fixes, there is still one remining
> libgomp failure in the testsuite when run on an HSA-enabled APU.  The
> problem is that grid calculation does not work with NE_EXPR conditions
> in omp loop constructs which is now permitted in OpenMP 5.
>
> The patch below fixes it by simply moving the code that deals with it
> into the function shared between omp expansion and gridification, and a
> place which also feels more natural, to omp_adjust_for_condition.  For
> some reason, this function is also called twice in omp_extract_for_data
> but the second call cannot have any effect, so I removed one.
>
> I have tested this on an HSA APU system with hsa offloading enabled and
> also bootstrapped and tested on a bigger x86_64-linux system.  OK for
> trunk?
>
> Thanks,
>
> Martin
>
>
> 2019-02-01  Martin Jambor  <mjambor@suse.cz>
>
> 	* omp-general.c (omp_extract_for_data): Removed a duplicate call
> 	to omp_adjust_for_condition, moved NE_EXPR code_cond processing...
> 	(omp_adjust_for_condition): ...here.  Added necessary parameters.
> 	* omp-general.h (omp_adjust_for_condition): Updated declaration.
> 	* omp-grid.c (grid_attempt_target_gridification): Adjust to pass
> 	proper values to new parameters of omp_adjust_for_condition.
> ---
>  gcc/omp-general.c | 67 ++++++++++++++++++++++++-----------------------
>  gcc/omp-general.h |  2 +-
>  gcc/omp-grid.c    |  9 ++++---
>  3 files changed, 40 insertions(+), 38 deletions(-)
>
> diff --git a/gcc/omp-general.c b/gcc/omp-general.c
> index 12210c556fc..0f66ba0c5d8 100644
> --- a/gcc/omp-general.c
> +++ b/gcc/omp-general.c
> @@ -56,18 +56,47 @@ omp_is_reference (tree decl)
>    return lang_hooks.decls.omp_privatize_by_reference (decl);
>  }
>  
> -/* Adjust *COND_CODE and *N2 so that the former is either LT_EXPR or
> -   GT_EXPR.  */
> +/* Adjust *COND_CODE and *N2 so that the former is either LT_EXPR or GT_EXPR,
> +   given that V is the loop index variable and STEP is loop step. */
>  
>  void
> -omp_adjust_for_condition (location_t loc, enum tree_code *cond_code, tree *n2)
> +omp_adjust_for_condition (location_t loc, enum tree_code *cond_code, tree *n2,
> +			  tree v, tree step)
>  {
>    switch (*cond_code)
>      {
>      case LT_EXPR:
>      case GT_EXPR:
> +      break;
> +
>      case NE_EXPR:
> +      gcc_assert (TREE_CODE (step) == INTEGER_CST);
> +      if (TREE_CODE (TREE_TYPE (v)) == INTEGER_TYPE)
> +	{
> +	  if (integer_onep (step))
> +	    *cond_code = LT_EXPR;
> +	  else
> +	    {
> +	      gcc_assert (integer_minus_onep (step));
> +	      *cond_code = GT_EXPR;
> +	    }
> +	}
> +      else
> +	{
> +	  tree unit = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (v)));
> +	  gcc_assert (TREE_CODE (unit) == INTEGER_CST);
> +	  if (tree_int_cst_equal (unit, step))
> +	    *cond_code = LT_EXPR;
> +	  else
> +	    {
> +	      gcc_assert (wi::neg (wi::to_widest (unit))
> +			  == wi::to_widest (step));
> +	      *cond_code = GT_EXPR;
> +	    }
> +	}
> +
>        break;
> +
>      case LE_EXPR:
>        if (POINTER_TYPE_P (TREE_TYPE (*n2)))
>  	*n2 = fold_build_pointer_plus_hwi_loc (loc, *n2, 1);
> @@ -258,41 +287,13 @@ omp_extract_for_data (gomp_for *for_stmt, struct omp_for_data *fd,
>        gcc_assert (loop->cond_code != NE_EXPR
>  		  || (gimple_omp_for_kind (for_stmt)
>  		      != GF_OMP_FOR_KIND_OACC_LOOP));
> -      omp_adjust_for_condition (loc, &loop->cond_code, &loop->n2);
>  
>        t = gimple_omp_for_incr (for_stmt, i);
>        gcc_assert (TREE_OPERAND (t, 0) == var);
>        loop->step = omp_get_for_step_from_incr (loc, t);
>  
> -      if (loop->cond_code == NE_EXPR)
> -	{
> -	  gcc_assert (TREE_CODE (loop->step) == INTEGER_CST);
> -	  if (TREE_CODE (TREE_TYPE (loop->v)) == INTEGER_TYPE)
> -	    {
> -	      if (integer_onep (loop->step))
> -		loop->cond_code = LT_EXPR;
> -	      else
> -		{
> -		  gcc_assert (integer_minus_onep (loop->step));
> -		  loop->cond_code = GT_EXPR;
> -		}
> -	    }
> -	  else
> -	    {
> -	      tree unit = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (loop->v)));
> -	      gcc_assert (TREE_CODE (unit) == INTEGER_CST);
> -	      if (tree_int_cst_equal (unit, loop->step))
> -		loop->cond_code = LT_EXPR;
> -	      else
> -		{
> -		  gcc_assert (wi::neg (wi::to_widest (unit))
> -			      == wi::to_widest (loop->step));
> -		  loop->cond_code = GT_EXPR;
> -		}
> -	    }
> -	}
> -
> -      omp_adjust_for_condition (loc, &loop->cond_code, &loop->n2);
> +      omp_adjust_for_condition (loc, &loop->cond_code, &loop->n2, loop->v,
> +				loop->step);
>  
>        if (simd
>  	  || (fd->sched_kind == OMP_CLAUSE_SCHEDULE_STATIC
> diff --git a/gcc/omp-general.h b/gcc/omp-general.h
> index f5f03c8b056..0cbbb31e73b 100644
> --- a/gcc/omp-general.h
> +++ b/gcc/omp-general.h
> @@ -73,7 +73,7 @@ struct omp_for_data
>  extern tree omp_find_clause (tree clauses, enum omp_clause_code kind);
>  extern bool omp_is_reference (tree decl);
>  extern void omp_adjust_for_condition (location_t loc, enum tree_code *cond_code,
> -				      tree *n2);
> +				      tree *n2, tree v, tree step);
>  extern tree omp_get_for_step_from_incr (location_t loc, tree incr);
>  extern void omp_extract_for_data (gomp_for *for_stmt, struct omp_for_data *fd,
>  				  struct omp_for_data_loop *loops);
> diff --git a/gcc/omp-grid.c b/gcc/omp-grid.c
> index 1fdd8fc2efb..1ff65aa160c 100644
> --- a/gcc/omp-grid.c
> +++ b/gcc/omp-grid.c
> @@ -1303,7 +1303,8 @@ grid_attempt_target_gridification (gomp_target *target,
>    push_gimplify_context ();
>    for (size_t i = 0; i < grid.collapse; i++)
>      {
> -      tree itype, type = TREE_TYPE (gimple_omp_for_index (inner_loop, i));
> +      tree index_var = gimple_omp_for_index (inner_loop, i);
> +      tree itype, type = TREE_TYPE (index_var);
>        if (POINTER_TYPE_P (type))
>  	itype = signed_type_for (type);
>        else
> @@ -1314,13 +1315,13 @@ grid_attempt_target_gridification (gomp_target *target,
>        walk_tree (&n1, grid_remap_prebody_decls, &wi, NULL);
>        tree n2 = unshare_expr (gimple_omp_for_final (inner_loop, i));
>        walk_tree (&n2, grid_remap_prebody_decls, &wi, NULL);
> -      omp_adjust_for_condition (loc, &cond_code, &n2);
> +      tree step
> +	= omp_get_for_step_from_incr (loc, gimple_omp_for_incr (inner_loop, i));
> +      omp_adjust_for_condition (loc, &cond_code, &n2, index_var, step);
>        n1 = fold_convert (itype, n1);
>        n2 = fold_convert (itype, n2);
>  
>        tree cond = fold_build2 (cond_code, boolean_type_node, n1, n2);
> -      tree step
> -	= omp_get_for_step_from_incr (loc, gimple_omp_for_incr (inner_loop, i));
>  
>        tree t = build_int_cst (itype, (cond_code == LT_EXPR ? -1 : 1));
>        t = fold_build2 (PLUS_EXPR, itype, step, t);
> -- 
> 2.20.1