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 842E83971C32 for ; Thu, 7 Jan 2021 07:59:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 842E83971C32 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 5B7AAAE39; Thu, 7 Jan 2021 07:59:25 +0000 (UTC) Date: Thu, 7 Jan 2021 08:59:25 +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 07:59:28 -0000 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). Thanks, Richard. > 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 | 9 ++++++++- > gcc/testsuite/gcc.dg/vect/pr98560-1.c | 17 +++++++++++++++++ > 2 files changed, 25 insertions(+), 1 deletion(-) > 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..9c07d79a86c 100644 > --- a/gcc/gimple-isel.cc > +++ b/gcc/gimple-isel.cc > @@ -254,7 +254,14 @@ 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)) > + && (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)