From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from fx403.security-mail.net (smtpout140.security-mail.net [85.31.212.143]) by sourceware.org (Postfix) with ESMTPS id 998A03857000 for ; Mon, 10 Oct 2022 15:20:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 998A03857000 Authentication-Results: sourceware.org; dmarc=fail (p=quarantine dis=none) header.from=kalrayinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kalray.eu Received: from localhost (localhost [127.0.0.1]) by fx403.security-mail.net (Postfix) with ESMTP id 2DEC7A0763E for ; Mon, 10 Oct 2022 17:20:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kalrayinc.com; s=sec-sig-email; t=1665415237; bh=Y+jDC4X3I3+sZliJsP2rgffi+ef2B1HPFXZpnEFAkO8=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=lERVSjw/f2bWLivB8xDzriEILXQS60L1qoirbfNwYbdlNZas6Os2F9DWMu7ce9l4f pZAzpO+vB2achSsgG1GIYab571SyNBbdkTJGCmw8GUgspV4isaVQ6bsHYfYFzs/WdG ZvrcQWVfVfch5ME5FOjvGo3IoETJGZwbH/x+VOfQ= Received: from fx403 (localhost [127.0.0.1]) by fx403.security-mail.net (Postfix) with ESMTP id 0EF92A0763B; Mon, 10 Oct 2022 17:20:37 +0200 (CEST) Received: from zimbra2.kalray.eu (unknown [217.181.231.53]) by fx403.security-mail.net (Postfix) with ESMTPS id 2113DA07627; Mon, 10 Oct 2022 17:20:36 +0200 (CEST) Received: from zimbra2.kalray.eu (localhost [127.0.0.1]) by zimbra2.kalray.eu (Postfix) with ESMTPS id F304627E044D; Mon, 10 Oct 2022 17:20:35 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by zimbra2.kalray.eu (Postfix) with ESMTP id D6C2527E044F; Mon, 10 Oct 2022 17:20:35 +0200 (CEST) Received: from zimbra2.kalray.eu ([127.0.0.1]) by localhost (zimbra2.kalray.eu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id NaCYnmCbkUAX; Mon, 10 Oct 2022 17:20:35 +0200 (CEST) Received: from localhost (unknown [192.168.37.51]) by zimbra2.kalray.eu (Postfix) with ESMTPSA id B4B2427E044D; Mon, 10 Oct 2022 17:20:35 +0200 (CEST) X-Virus-Scanned: E-securemail Secumail-id: <107de.63443844.1fe35.0> DKIM-Filter: OpenDKIM Filter v2.10.3 zimbra2.kalray.eu D6C2527E044F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kalrayinc.com; s=4F334102-7B72-11EB-A74E-42D0B9747555; t=1665415235; bh=DU0AriGPWFM9PfEFa+1bkFJmNIoWB+qefEnEzOr27bM=; h=Date:From:To:Message-ID:MIME-Version; b=VRVZRNTWNHwnLZ/MSaqZ5qzJ4q3vabTdgOyROiU7VEfxEj0x6jHEGwZADLDtKUBZ6 Wh7bduyC62hLCAfquU7rXGwSbP9u3fdBLRyD2mYIJjmGc2Ox6gjcY7DXBeWcsEsJ5q AWqeQwS0U/oWXb+uduR/jUPhiOfhGrkpYvM+NpZpA6Xoi2jT6d2S/FQmhacO8Nu+71 /TzX9dNAqBKcFoBiFj2a5uLvqlcpgchBSjHuoPiYItwuwvA75MzbtY1oSHerE3MVzl NIbF7J01yvWea6LADTxuDuTZqAqQRZ3A4Y1P0Vn3XtG6N+eI+CffORgeHy1df+SgrL 6glgyxhmCowRQ== Date: Mon, 10 Oct 2022 17:20:20 +0200 From: Paul Iannetta To: Iain Sandoe Cc: GCC Patches , Joseph Myers Subject: Re: [RFC] Add support for vectors in comparisons (like the C++ frontend does) Message-ID: <20221010152020.fwyaggd6ndzy7gyb@ws2202.lin.mbt.kalray.eu> References: <20221010141141.krpmtzmbgadlo3db@ws2202.lin.mbt.kalray.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20171215 Content-Transfer-Encoding: 8bit X-ALTERMIMEV2_out: done X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Ian, Observations are also very welcomed, thank you! On Mon, Oct 10, 2022 at 03:37:24PM +0100, Iain Sandoe wrote: > 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 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? > The main motivation behind the equivalence is that, we have C and C++ codebases, and it is not very convenient to have to remember which extension is allowed in C and not in C++ and vice-versa. And, in this case, it makes it harder for GCC to generate conditional moves for code using vectors, especially since `a ? b : c` is not recognized, and thus, we cannot rely on the `VEC_COND_EXPR` construction. > > 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). Well, the only motivation was that it was easier to make the change for both front-ends at the same time and avoided some checks. I'll add them so that it won't introduce a divergence on what is accepted by the objc front-end. Thanks, Paul > > 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}, > > > > > > > > > > > > >