From: Iain Sandoe <iain@sandoe.co.uk>
To: Paul Iannetta <piannetta@kalrayinc.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
Joseph Myers <joseph@codesourcery.com>
Subject: Re: [RFC] Add support for vectors in comparisons (like the C++ frontend does)
Date: Mon, 10 Oct 2022 15:37:24 +0100 [thread overview]
Message-ID: <E7E67D92-C698-4697-A88D-F170EEEDC761@sandoe.co.uk> (raw)
In-Reply-To: <20221010141141.krpmtzmbgadlo3db@ws2202.lin.mbt.kalray.eu>
Hi Paul,
Not a review of the patch - but a couple of observations.
> On 10 Oct 2022, at 15:11, Paul Iannetta via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 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.
Equivalence seems, on the face of it, a reasonable objective - but I am
curious as whether there is some more concrete motivation for the patch,
e.g. some codebase that currently does not work but would with this change?
> 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.
Likewise, I am interested in the motivation for the ObjC change. The usual
initial filter for consideration of functional changes (at least for the NeXT
runtime on Darwin) is “What does current clang do?” or, even better, “what
does current Xcode do?”. There are several (possibly many) cases where
C in clang has extensions to C++ behaviour - and vice-versa (because there
is a single front-end codebase, that happens easily, whether by design o
accident).
The reason for the ‘clang litmus test’ is that the effective language standard
for ObjectiveC is determined by that compiler.
cheers
Iain
> 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.
>
> 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},
>
>
>
>
next prev parent reply other threads:[~2022-10-10 14:37 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 [this message]
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
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=E7E67D92-C698-4697-A88D-F170EEEDC761@sandoe.co.uk \
--to=iain@sandoe.co.uk \
--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).