From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 4D1BA386F030 for ; Thu, 7 Jan 2021 13:13:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4D1BA386F030 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rguenther@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 7AB3AB716; Thu, 7 Jan 2021 13:13:04 +0000 (UTC) Date: Thu, 7 Jan 2021 14:13:04 +0100 (CET) From: Richard Biener To: Richard Sandiford cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] gimple-isel: Fall back to using vcond_mask [PR98560] In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2021 13:13:08 -0000 On Thu, 7 Jan 2021, Richard Sandiford wrote: > Richard Biener writes: > > On Wed, 6 Jan 2021, Richard Sandiford wrote: > > > >> PR98560 is about a case in which the vectoriser initially generates: > >> > >> mask_1 = a < 0; > >> mask_2 = mask_1 & ...; > >> res = VEC_COND_EXPR ; > >> > >> The vectoriser thus expects res to be calculated using vcond_mask. > >> However, we later manage to fold mask_2 to mask_1, leaving: > >> > >> mask_1 = a < 0; > >> res = VEC_COND_EXPR ; > >> > >> gimple-isel then required a combined vcond to exist. > >> > >> On most targets, it's not too onerous to provide all possible > >> (compare x select) combinations. For each data mode, you just > >> need to provide unsigned comparisons, signed comparisons, and > >> floating-point comparisons, with the data mode and type of > >> comparison uniquely determining the mode of the compared values. > >> But for targets like SVE that support “unpacked” vectors, > >> it's not that simple: the level of unpacking adds another > >> degree of freedom. > >> > >> Rather than insist that the combined versions exist, I think > >> we should be prepared to fall back to using separate comparisons > >> and vcond_masks. I think that makes more sense on targets like > >> AArch64 and AArch32 in which compares and selects are fundementally > >> separate operations anyway. > > > > Indeed the mask variants (thus being able to expand the comparison) > > are more fundamental. I guess you're running into this path because > > we did not consider using vcond_mask because of > > > > if (used_vec_cond_exprs >= 2 > > && (get_vcond_mask_icode (mode, TYPE_MODE (op0_type)) > > != CODE_FOR_nothing) > > && expand_vec_cmp_expr_p (op0a_type, op0_type, tcode)) > > { > > /* Keep the SSA name and use vcond_mask. */ > > tcode = TREE_CODE (op0); > > } > > > > not triggering? Which also means your patch fails to check/assert > > that we can expand_vec_cmp_expr_p the separate compare? > > > >> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > > > > It does feel like the function could need some refactoring ... > > > > But OK - preferably with the assertion that we can actually > > expand the compare (I suggest to do the expand_vec_cmp_expr_p > > above unconditionally and have a 'global' cannot_expand_mask > > flag defaulted to false and checked in the new path). > > OK, how does this look? It's not quite what you said because > I wanted to avoid the double negation in !cannot. LGTM. Thanks, Richard. > Thanks, > Richard > > > gcc/ > PR tree-optimization/98560 > * gimple-isel.cc (gimple_expand_vec_cond_expr): If we fail to use > IFN_VCOND{,U,EQ}, fall back on IFN_VCOND_MASK. > > gcc/testsuite/ > PR tree-optimization/98560 > * gcc.dg/vect/pr98560-1.c: New test. > --- > gcc/gimple-isel.cc | 26 +++++++++++++++++++------- > gcc/testsuite/gcc.dg/vect/pr98560-1.c | 17 +++++++++++++++++ > 2 files changed, 36 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/vect/pr98560-1.c > > diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc > index d40338ce4a2..0f3d6bba229 100644 > --- a/gcc/gimple-isel.cc > +++ b/gcc/gimple-isel.cc > @@ -154,6 +154,7 @@ gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi, > return gimple_build_assign (lhs, tem3); > } > > + bool can_compute_op0 = true; > gcc_assert (!COMPARISON_CLASS_P (op0)); > if (TREE_CODE (op0) == SSA_NAME) > { > @@ -184,13 +185,16 @@ gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi, > > tree op0_type = TREE_TYPE (op0); > tree op0a_type = TREE_TYPE (op0a); > + if (TREE_CODE_CLASS (tcode) == tcc_comparison) > + can_compute_op0 = expand_vec_cmp_expr_p (op0a_type, op0_type, > + tcode); > > /* Try to fold x CMP y ? -1 : 0 to x CMP y. */ > > - if (integer_minus_onep (op1) > + if (can_compute_op0 > + && integer_minus_onep (op1) > && integer_zerop (op2) > - && TYPE_MODE (TREE_TYPE (lhs)) == TYPE_MODE (TREE_TYPE (op0)) > - && expand_vec_cmp_expr_p (op0a_type, op0_type, tcode)) > + && TYPE_MODE (TREE_TYPE (lhs)) == TYPE_MODE (TREE_TYPE (op0))) > { > tree conv_op = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs), op0); > gassign *new_stmt = gimple_build_assign (lhs, conv_op); > @@ -198,10 +202,10 @@ gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi, > return new_stmt; > } > > - if (used_vec_cond_exprs >= 2 > + if (can_compute_op0 > + && used_vec_cond_exprs >= 2 > && (get_vcond_mask_icode (mode, TYPE_MODE (op0_type)) > - != CODE_FOR_nothing) > - && expand_vec_cmp_expr_p (op0a_type, op0_type, tcode)) > + != CODE_FOR_nothing)) > { > /* Keep the SSA name and use vcond_mask. */ > tcode = TREE_CODE (op0); > @@ -254,7 +258,15 @@ gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi, > } > } > > - gcc_assert (icode != CODE_FOR_nothing); > + if (icode == CODE_FOR_nothing) > + { > + gcc_assert (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (op0)) > + && can_compute_op0 > + && (get_vcond_mask_icode (mode, TYPE_MODE (TREE_TYPE (op0))) > + != CODE_FOR_nothing)); > + return gimple_build_call_internal (IFN_VCOND_MASK, 3, op0, op1, op2); > + } > + > tree tcode_tree = build_int_cst (integer_type_node, tcode); > return gimple_build_call_internal (unsignedp ? IFN_VCONDU : IFN_VCOND, > 5, op0a, op0b, op1, op2, tcode_tree); > diff --git a/gcc/testsuite/gcc.dg/vect/pr98560-1.c b/gcc/testsuite/gcc.dg/vect/pr98560-1.c > new file mode 100644 > index 00000000000..2583fc48f8a > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/pr98560-1.c > @@ -0,0 +1,17 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-O3 -fno-tree-vrp -fno-tree-fre -fno-tree-pre -fno-code-hoisting -fvect-cost-model=dynamic" } */ > +/* { dg-additional-options "-msve-vector-bits=128" { target aarch64_sve } } */ > + > +#include > + > +void > +f (uint16_t *restrict dst, uint32_t *restrict src1, float *restrict src2) > +{ > + int i = 0; > + for (int j = 0; j < 4; ++j) > + { > + uint16_t tmp = src1[i] >> 1; > + dst[i] = (uint16_t) (src2[i] < 0 && i < 4 ? tmp : 1); > + i += 1; > + } > +} > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)