On Sat, May 01, 2021 at 01:12:15AM +0200, Tobias Burnus wrote:
> gcc/c/ChangeLog:
> 
>       * c-typeck.c (c_finish_omp_clauses): Accept float + complex for || and 
> &&
>       reductions.
> 
> gcc/cp/ChangeLog:
> 
>       * semantics.c (finish_omp_reduction_clause): Accept float + complex for 
> || and &&
>       reductions.
> 
> gcc/ChangeLog:
> 
>       * omp-low.c (lower_rec_input_clauses, lower_reduction_clauses): Handle 
> && and ||
>       with floating-point and complex arguments.

All the above ChangeLog lines are too long.

> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -6376,6 +6376,11 @@ lower_rec_input_clauses (tree clauses, gimple_seq 
> *ilist, gimple_seq *dlist,
>                 if (code == MINUS_EXPR)
>                   code = PLUS_EXPR;
>  
> +               /* C/C++ permits FP/complex with || and &&.  */
> +               bool is_fp_and_or
> +                 = ((code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR)
> +                    && (FLOAT_TYPE_P (TREE_TYPE (new_var))
> +                        || TREE_CODE (TREE_TYPE (new_var)) == COMPLEX_TYPE));

The above line is too long too, please use
                           || (TREE_CODE (TREE_TYPE (new_var))
                               == COMPLEX_TYPE)));

>                 tree new_vard = new_var;
>                 if (is_simd && omp_is_reference (var))
>                   {
> @@ -6443,8 +6448,23 @@ lower_rec_input_clauses (tree clauses, gimple_seq 
> *ilist, gimple_seq *dlist,
>                     if (is_simd)
>                       {
>                         tree ref = build_outer_var_ref (var, ctx);
> -
> -                       x = build2 (code, TREE_TYPE (ref), ref, new_var);
> +                       tree new_var2 = new_var;
> +                       if (is_fp_and_or)
> +                         new_var2 = fold_build2_loc (
> +                                      clause_loc, NE_EXPR,
> +                                      integer_type_node, new_var,
> +                                      build_zero_cst (TREE_TYPE (new_var)));

Formatting, would be nice to avoid the ( at the end of line, e.g.
                            {
                              tree zero = build_zero_cst (TREE_TYPE (new_var));
                              new_var2
                                = fold_build2_loc (clause_loc, NE_EXPR,
                                                   integer_type_node, new_var,
                                                   zero);
                            }

> +                       tree ref2 = ref;
> +                       if (is_fp_and_or
> +                           && (FLOAT_TYPE_P (TREE_TYPE (ref))
> +                               || TREE_CODE (TREE_TYPE (ref))
> +                                  == COMPLEX_TYPE))

Please wrap the == into ()s.
Though ref should have the same type new_var (or at least a compatible type),
so I don't see the point of the && ... in there and of using two separate
if (is_fp_and_or) blocks.
So
tree new_var2 = new_var;
tree ref2 = ref;
if (is_fp_and_or)
  {
    tree zero = ...;
    new_var2 = ...
    ref2 = ...;
  }

> +                         ref2 = fold_build2_loc (
> +                                      clause_loc, NE_EXPR, integer_type_node,
> +                                      ref, build_zero_cst (TREE_TYPE (ref)));

And try to avoid the ( here too.
Even better would be to split the function a little bit, but that can be
done another day.

> +                       x = build2 (code, TREE_TYPE (ref2), ref2, new_var2);
> +                       if (new_var2 != new_var)
> +                         x = fold_convert (TREE_TYPE (new_var), x);
>                         ref = build_outer_var_ref (var, ctx);
>                         gimplify_assign (ref, x, dlist);
>                       }
> @@ -7384,13 +7404,32 @@ lower_reduction_clauses (tree clauses, gimple_seq 
> *stmt_seqp,
>        if (code == MINUS_EXPR)
>          code = PLUS_EXPR;
>  
> +      /* C/C++ permits FP/complex with || and &&.  */
> +      bool is_fp_and_or = ((code == TRUTH_ANDIF_EXPR || code == 
> TRUTH_ORIF_EXPR)
> +                        && (FLOAT_TYPE_P (TREE_TYPE (new_var))
> +                            || TREE_CODE (TREE_TYPE (new_var))
> +                                == COMPLEX_TYPE));

Again, ()s around ==.

>        if (count == 1)
>       {
>         tree addr = build_fold_addr_expr_loc (clause_loc, ref);
>  
>         addr = save_expr (addr);
>         ref = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (addr)), addr);
> -       x = fold_build2_loc (clause_loc, code, TREE_TYPE (ref), ref, new_var);
> +       tree new_var2 = new_var;
> +       if (is_fp_and_or)
> +         new_var2 = fold_build2_loc (clause_loc, NE_EXPR,
> +                                     integer_type_node, new_var,
> +                                     build_zero_cst (TREE_TYPE (new_var)));
> +       tree ref2 = ref;
> +       if (is_fp_and_or
> +           && (FLOAT_TYPE_P (TREE_TYPE (ref))
> +               || TREE_CODE (TREE_TYPE (ref)) == COMPLEX_TYPE))
> +         ref2 = fold_build2_loc (clause_loc, NE_EXPR, integer_type_node, ref,
> +                                 build_zero_cst (TREE_TYPE (ref)));

And similar question as above.
And the line is too long.

> +       x = fold_build2_loc (clause_loc, code, TREE_TYPE (new_var2), ref2,
> +                            new_var2);
> +       if (new_var2 != new_var)
> +         x = fold_convert (TREE_TYPE (new_var), x);
>         x = build2 (OMP_ATOMIC, void_type_node, addr, x);
>         OMP_ATOMIC_MEMORY_ORDER (x) = OMP_MEMORY_ORDER_RELAXED;
>         gimplify_and_add (x, stmt_seqp);
> @@ -7495,7 +7534,21 @@ lower_reduction_clauses (tree clauses, gimple_seq 
> *stmt_seqp,
>           }
>         else
>           {
> -           x = build2 (code, TREE_TYPE (out), out, priv);
> +           tree out2 = out;
> +           if (is_fp_and_or)
> +             out2 = fold_build2_loc (clause_loc, NE_EXPR,
> +                                     integer_type_node, out,
> +                                     build_zero_cst (type));
> +           tree priv2 = priv;
> +           if (is_fp_and_or
> +               && (FLOAT_TYPE_P (TREE_TYPE (priv))
> +                   || TREE_CODE (TREE_TYPE (priv)) == COMPLEX_TYPE))

And here too.

> +             priv2 = fold_build2_loc (clause_loc, NE_EXPR,
> +                                      integer_type_node, priv,
> +                                      build_zero_cst (TREE_TYPE (priv)));
> +           x = build2 (code, TREE_TYPE (out2), out2, priv2);
> +           if (out2 != out)
> +             x = fold_convert (TREE_TYPE (out), x);
>             out = unshare_expr (out);
>             gimplify_assign (out, x, &sub_seq);
>           }
> @@ -7529,7 +7582,20 @@ lower_reduction_clauses (tree clauses, gimple_seq 
> *stmt_seqp,
>       }
>        else
>       {
> -       x = build2 (code, TREE_TYPE (ref), ref, new_var);
> +       tree new_var2 = new_var;
> +       if (is_fp_and_or)
> +         new_var2 = fold_build2_loc (clause_loc, NE_EXPR,
> +                                     integer_type_node, new_var,
> +                                     build_zero_cst (TREE_TYPE (new_var)));
> +       tree ref2 = ref;
> +       if (is_fp_and_or
> +           && (FLOAT_TYPE_P (TREE_TYPE (ref))
> +               || TREE_CODE (TREE_TYPE (ref)) == COMPLEX_TYPE))
> +         ref2 = fold_build2_loc (clause_loc, NE_EXPR, integer_type_node, ref,
> +                                     build_zero_cst (TREE_TYPE (ref)));
> +       x = build2 (code, TREE_TYPE (ref), ref2, new_var2);
> +       if (new_var2 != new_var)
> +         x = fold_convert (TREE_TYPE (new_var), x);

Likewise.

For the testcases, would be nice to have one with _Complex int, though
perhaps separately from the ones you've included because while float
or _Complex double are standard, _Complex int is a GNU extension.

Otherwise LGTM.

        Jakub

Reply via email to