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