public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Paul Iannetta <piannetta@kalrayinc.com>
Cc: gcc-patches@gcc.gnu.org, joseph@codesourcery.com
Subject: Re: [RFC] Add support for vectors in comparisons (like the C++ frontend does)
Date: Tue, 11 Oct 2022 08:53:59 +0200	[thread overview]
Message-ID: <CAFiYyc1+UQb5otOP76cbj3ZHb41rQ3wUdHRL_5D1hivhEw0Y8g@mail.gmail.com> (raw)
In-Reply-To: <20221010141141.krpmtzmbgadlo3db@ws2202.lin.mbt.kalray.eu>

On Mon, Oct 10, 2022 at 4:12 PM Paul Iannetta via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
>
> I am trying to bridge the gap between the extensions supported by the C
> and C++ front-ends.  When it comes to vector extensions, C++ supports
> vectors in comparisons and within conditional expressions whereas the C
> front-end does not.
>
> I have a patch to bring this feature to the C front-end as well, and
> would like to hear your opinion on it, especially since it may affect
> the feature-set of the objc front-end as well.
>
> I have tried to mirror as much as possible what the C++ front-end does
> and checked that both front-end produce the same GIMPLE for all the
> simple expressions as well as some more complex combinations of the
> operators (?:, !, ^, || and &&).
>
> Currently, this is only a tentative patch and I did not add any tests
> to the testsuite.  Moreover, the aarch64's target-specific testsuite
> explicitly tests the non-presence of this feature, which will have to
> be removed.
>
> I've run the testsuite on x86 and I've not seen any regressions.

As Joseph says testcases for this are necessary, possibly by moving
existing tests to c-c++-common.  Otherwise I welcome this change,
the divergence in features has been annoying at least.

Richard.

> Cheers,
> Paul
>
> # ------------------------ >8 ------------------------
> Support for vector types in simple comparisons
>
> gcc/
>
>         * doc/extend.texi: Remove the C++ mention, since both C and C++
>           support the all the mentioned features.
>
> gcc/c/
>
>         * c-typeck.cc (build_unary_op): Add support for vector for the
>           unary exclamation mark.
>         (build_conditional_expr): Add support for vector in conditional
>         expressions.
>         (build_binary_op): Add support for vector for &&, || and ^.
>         (c_objc_common_truthvalue_conversion): Remove the special gards
>         preventing vector types.
>
> # ------------------------ >8 ------------------------
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index 17185fd3da4..03ade14cae9 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -4536,12 +4536,15 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
>      case TRUTH_NOT_EXPR:
>        if (typecode != INTEGER_TYPE && typecode != FIXED_POINT_TYPE
>           && typecode != REAL_TYPE && typecode != POINTER_TYPE
> -         && typecode != COMPLEX_TYPE)
> +         && typecode != COMPLEX_TYPE && typecode != VECTOR_TYPE)
>         {
>           error_at (location,
>                     "wrong type argument to unary exclamation mark");
>           return error_mark_node;
>         }
> +      if (gnu_vector_type_p (TREE_TYPE (arg)))
> +       return build_binary_op (location, EQ_EXPR, arg,
> +                               build_zero_cst (TREE_TYPE (arg)), false);
>        if (int_operands)
>         {
>           arg = c_objc_common_truthvalue_conversion (location, xarg);
> @@ -5477,6 +5480,129 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
>        result_type = type2;
>      }
>
> +  if (gnu_vector_type_p (TREE_TYPE (ifexp))
> +      && VECTOR_INTEGER_TYPE_P (TREE_TYPE (ifexp)))
> +    {
> +      tree ifexp_type = TREE_TYPE (ifexp);
> +
> +      /* If ifexp is another cond_expr choosing between -1 and 0,
> +        then we can use its comparison.  It may help to avoid
> +        additional comparison, produce more accurate diagnostics
> +        and enables folding.  */
> +      if (TREE_CODE (ifexp) == VEC_COND_EXPR
> +         && integer_minus_onep (TREE_OPERAND (ifexp, 1))
> +         && integer_zerop (TREE_OPERAND (ifexp, 2)))
> +       ifexp = TREE_OPERAND (ifexp, 0);
> +
> +      tree op1_type = TREE_TYPE (op1);
> +      tree op2_type = TREE_TYPE (op2);
> +
> +      if (!VECTOR_TYPE_P (op1_type) && !VECTOR_TYPE_P (op2_type))
> +       {
> +         /* Rely on the error messages of the scalar version.  */
> +         tree scal =
> +           build_conditional_expr (colon_loc, integer_one_node, ifexp_bcp,
> +                                   op1, op1_original_type, op1_loc,
> +                                   op2, op2_original_type, op2_loc);
> +         if (scal == error_mark_node)
> +           return error_mark_node;
> +         tree stype = TREE_TYPE (scal);
> +         tree ctype = TREE_TYPE (ifexp_type);
> +         if (TYPE_SIZE (stype) != TYPE_SIZE (ctype)
> +             || (!INTEGRAL_TYPE_P (stype) && !SCALAR_FLOAT_TYPE_P (stype)))
> +           {
> +             error_at (colon_loc,
> +                       "inferred scalar type %qT is not an integer or "
> +                       "floating-point type of the same size as %qT", stype,
> +                       COMPARISON_CLASS_P (ifexp)
> +                       ? TREE_TYPE (TREE_TYPE (TREE_OPERAND (ifexp, 0)))
> +                       : ctype);
> +             return error_mark_node;
> +           }
> +
> +         tree vtype = build_opaque_vector_type (stype,
> +                                                TYPE_VECTOR_SUBPARTS
> +                                                (ifexp_type));
> +         /* The warnings (like Wsign-conversion) have already been
> +            given by the scalar build_conditional_expr. We still check
> +            unsafe_conversion_p to forbid truncating long long -> float.  */
> +         if (unsafe_conversion_p (stype, op1, NULL_TREE, false))
> +           {
> +             error_at (colon_loc, "conversion of scalar %qT to vector %qT "
> +                       "involves truncation", op1_type, vtype);
> +             return error_mark_node;
> +           }
> +         if (unsafe_conversion_p (stype, op2, NULL_TREE, false))
> +           {
> +             error_at (colon_loc, "conversion of scalar %qT to vector %qT "
> +                       "involves truncation", op2_type, vtype);
> +             return error_mark_node;
> +           }
> +
> +         op1 = convert (stype, op1);
> +         op1 = save_expr (op1);
> +         op1 = build_vector_from_val (vtype, op1);
> +         op1_type = vtype;
> +         op2 = convert (stype, op2);
> +         op2 = save_expr (op2);
> +         op2 = build_vector_from_val (vtype, op2);
> +         op2_type = vtype;
> +       }
> +
> +      if (gnu_vector_type_p (op1_type) ^ gnu_vector_type_p (op2_type))
> +       {
> +         enum stv_conv convert_flag =
> +           scalar_to_vector (colon_loc, VEC_COND_EXPR, op1, op2,
> +                             true);
> +
> +         switch (convert_flag)
> +           {
> +           case stv_error:
> +             return error_mark_node;
> +           case stv_firstarg:
> +             {
> +               op1 = save_expr (op1);
> +               op1 = convert (TREE_TYPE (op2_type), op1);
> +               op1 = build_vector_from_val (op2_type, op1);
> +               op1_type = TREE_TYPE (op1);
> +               break;
> +             }
> +           case stv_secondarg:
> +             {
> +               op2 = save_expr (op2);
> +               op2 = convert (TREE_TYPE (op1_type), op2);
> +               op2 = build_vector_from_val (op1_type, op2);
> +               op2_type = TREE_TYPE (op2);
> +               break;
> +             }
> +           default:
> +             break;
> +           }
> +       }
> +
> +      if (!gnu_vector_type_p (op1_type)
> +         || !gnu_vector_type_p (op2_type)
> +         || !comptypes (op1_type, op2_type)
> +         || maybe_ne (TYPE_VECTOR_SUBPARTS (ifexp_type),
> +                      TYPE_VECTOR_SUBPARTS (op1_type))
> +         || TYPE_SIZE (ifexp_type) != TYPE_SIZE (op1_type))
> +       {
> +         error_at (colon_loc,
> +                   "incompatible vector types in conditional expression: "
> +                   "%qT, %qT and %qT", TREE_TYPE (ifexp),
> +                   TREE_TYPE (orig_op1), TREE_TYPE (orig_op2));
> +         return error_mark_node;
> +       }
> +
> +      if (!COMPARISON_CLASS_P (ifexp))
> +       {
> +         tree cmp_type = truth_type_for (ifexp_type);
> +         ifexp = build2 (NE_EXPR, cmp_type, ifexp,
> +                         build_zero_cst (ifexp_type));
> +       }
> +      return build3_loc (colon_loc, VEC_COND_EXPR, op1_type, ifexp, op1, op2);
> +    }
> +
>    if (!result_type)
>      {
>        if (flag_cond_mismatch)
> @@ -5522,17 +5648,6 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
>                        && !TREE_OVERFLOW (orig_op2)));
>      }
>
> -  /* Need to convert condition operand into a vector mask.  */
> -  if (VECTOR_TYPE_P (TREE_TYPE (ifexp)))
> -    {
> -      tree vectype = TREE_TYPE (ifexp);
> -      tree elem_type = TREE_TYPE (vectype);
> -      tree zero = build_int_cst (elem_type, 0);
> -      tree zero_vec = build_vector_from_val (vectype, zero);
> -      tree cmp_type = truth_type_for (vectype);
> -      ifexp = build2 (NE_EXPR, cmp_type, ifexp, zero_vec);
> -    }
> -
>    if (int_const || (ifexp_bcp && TREE_CODE (ifexp) == INTEGER_CST))
>      ret = fold_build3_loc (colon_loc, COND_EXPR, result_type, ifexp, op1, op2);
>    else
> @@ -12105,6 +12220,54 @@ build_binary_op (location_t location, enum tree_code code,
>                        && (op0 == truthvalue_true_node
>                            || !TREE_OVERFLOW (orig_op1)));
>         }
> +      if (!VECTOR_TYPE_P (type0) && gnu_vector_type_p (type1))
> +       {
> +         if (!COMPARISON_CLASS_P (op1))
> +           op1 = build_binary_op (EXPR_LOCATION (op1), NE_EXPR, op1,
> +                                  build_zero_cst (type1), false);
> +         if (code == TRUTH_ANDIF_EXPR)
> +           {
> +             tree z = build_zero_cst (TREE_TYPE (op1));
> +             return build_conditional_expr (location, op0, 0,
> +                                            op1, NULL_TREE, EXPR_LOCATION (op1),
> +                                            z, NULL_TREE, EXPR_LOCATION (z));
> +           }
> +         else if (code == TRUTH_ORIF_EXPR)
> +           {
> +             tree m1 = build_all_ones_cst (TREE_TYPE (op1));
> +             return build_conditional_expr (location, op0, 0,
> +                                             m1, NULL_TREE, EXPR_LOCATION (m1),
> +                                             op1, NULL_TREE, EXPR_LOCATION (op1));
> +           }
> +         else
> +           gcc_unreachable ();
> +       }
> +      if (gnu_vector_type_p (type0)
> +         && (!VECTOR_TYPE_P (type1) || gnu_vector_type_p (type1)))
> +       {
> +         if (!COMPARISON_CLASS_P (op0))
> +           op0 = build_binary_op (EXPR_LOCATION (op0), NE_EXPR, op0,
> +                                     build_zero_cst (type0), false);
> +         if (!VECTOR_TYPE_P (type1))
> +           {
> +             tree m1 = build_all_ones_cst (TREE_TYPE (op0));
> +             tree z = build_zero_cst (TREE_TYPE (op0));
> +             op1 = build_conditional_expr (location, op1, 0,
> +                                           m1, NULL_TREE, EXPR_LOCATION (m1),
> +                                           z, NULL_TREE, EXPR_LOCATION(z));
> +           }
> +         else if (!COMPARISON_CLASS_P (op1))
> +           op1 = build_binary_op (EXPR_LOCATION (op1), NE_EXPR, op1,
> +                                  build_zero_cst (type1), false);
> +         if (code == TRUTH_ANDIF_EXPR)
> +           code = BIT_AND_EXPR;
> +         else if (code == TRUTH_ORIF_EXPR)
> +           code = BIT_IOR_EXPR;
> +         else
> +           gcc_unreachable ();
> +
> +         return build_binary_op (location, code, op0, op1, false);
> +       }
>        break;
>
>        /* Shift operations: result has same type as first operand;
> @@ -12906,10 +13069,6 @@ c_objc_common_truthvalue_conversion (location_t location, tree expr)
>      case FUNCTION_TYPE:
>        gcc_unreachable ();
>
> -    case VECTOR_TYPE:
> -      error_at (location, "used vector type where scalar is required");
> -      return error_mark_node;
> -
>      default:
>        break;
>      }
> @@ -12924,8 +13083,6 @@ c_objc_common_truthvalue_conversion (location_t location, tree expr)
>        expr = note_integer_operands (expr);
>      }
>    else
> -    /* ??? Should we also give an error for vectors rather than leaving
> -       those to give errors later?  */
>      expr = c_common_truthvalue_conversion (location, expr);
>
>    if (TREE_CODE (expr) == INTEGER_CST && int_operands && !int_const)
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index c89df8778b2..1e0d436c02c 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -12007,7 +12007,7 @@ c = a >  b;     /* The result would be @{0, 0,-1, 0@}  */
>  c = a == b;     /* The result would be @{0,-1, 0,-1@}  */
>  @end smallexample
>
> -In C++, the ternary operator @code{?:} is available. @code{a?b:c}, where
> +The ternary operator @code{?:} is available. @code{a?b:c}, where
>  @code{b} and @code{c} are vectors of the same type and @code{a} is an
>  integer vector with the same number of elements of the same size as @code{b}
>  and @code{c}, computes all three arguments and creates a vector
> @@ -12020,7 +12020,7 @@ vector. If both @code{b} and @code{c} are scalars and the type of
>  @code{b} and @code{c} are converted to a vector type whose elements have
>  this type and with the same number of elements as @code{a}.
>
> -In C++, the logic operators @code{!, &&, ||} are available for vectors.
> +The logic operators @code{!, &&, ||} are available for vectors.
>  @code{!v} is equivalent to @code{v == 0}, @code{a && b} is equivalent to
>  @code{a!=0 & b!=0} and @code{a || b} is equivalent to @code{a!=0 | b!=0}.
>  For mixed operations between a scalar @code{s} and a vector @code{v},
>
>
>
>

      parent reply	other threads:[~2022-10-11  6:54 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-10 14:11 Paul Iannetta
2022-10-10 14:37 ` Iain Sandoe
2022-10-10 15:20   ` Paul Iannetta
2022-10-10 15:39     ` Iain Sandoe
2022-10-10 23:07 ` Joseph Myers
2022-10-11 23:18   ` Paul Iannetta
2022-10-14 14:17     ` Paul Iannetta
2022-10-17  7:22       ` Richard Biener
2022-10-18  9:21         ` Paul Iannetta
2022-10-11  6:53 ` Richard Biener [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFiYyc1+UQb5otOP76cbj3ZHb41rQ3wUdHRL_5D1hivhEw0Y8g@mail.gmail.com \
    --to=richard.guenther@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=joseph@codesourcery.com \
    --cc=piannetta@kalrayinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).