From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id 069FE3858D39 for ; Wed, 2 Aug 2023 07:49:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 069FE3858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id A8A5721B25; Wed, 2 Aug 2023 07:49:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1690962572; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=0GNxlKSzD/OYC9R4DOzJ/7tZPW891ay1mrzn+TOcbBw=; b=W6PdsYP55ox3TSGLcX2EAV1X1HPYgjr0H3pzijjyQQt8JDy6uOykwJtpZYgYA7g8xxWRpI AqIQpHdzhl0BL1TaNuvqE04IR8wYwd9keY72LBXrsmOjyZhv2IZRg4Y9JugDs9fGrtIg1i pHCbDXyyMo5H+yNigbW93l5qt2LC29c= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1690962572; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=0GNxlKSzD/OYC9R4DOzJ/7tZPW891ay1mrzn+TOcbBw=; b=xSEcJ6nraCEIflQgyskSpjgSwf7PCJjVJItYjlHaj318zCCJCXHetFZIjBXOoht+iivQG0 3O3QvIWzNSrrA8DQ== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 78BAC2C142; Wed, 2 Aug 2023 07:49:31 +0000 (UTC) Date: Wed, 2 Aug 2023 07:49:31 +0000 (UTC) From: Richard Biener To: =?GB2312?B?1tO+09Xc?= cc: "richard.sandiford" , gcc-patches Subject: Re: Re: [PATCH V2] VECT: Support CALL vectorization for COND_LEN_* In-Reply-To: <9C44AF4ECAAE18FE+2023073122291473567583@rivai.ai> Message-ID: References: <20230728071039.107552-1-juzhe.zhong@rivai.ai>, , , , <563EF72699BF4496+2023073120092631361920@rivai.ai>, , , <9C44AF4ECAAE18FE+2023073122291473567583@rivai.ai> User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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: On Mon, 31 Jul 2023, ??? wrote: > Oh, Thanks a lot. > I can test it in RISC-V backend now. > > But I have another questions: > >> I'm a bit confused (but also by the existing mask code), whether > >>vect_nargs needs adjustment depends on the IFN in the IL we analyze. > >>If if-conversion recognizes a .COND_ADD then we need to add nothing > >>for masking (that is, ifn == cond_fn already). In your code above > >>you either use cond_len_fn or get_len_internal_fn (cond_fn) but > >>isn't that the very same?! So how come you in one case add two > >>and in the other add four args? > >>Please make sure to place gcc_unreachable () in each arm and check > >>you have test coverage. I believe that the else arm is unreachable > >>but when you vectorize .FMA you will need to add 4 and when you > >>vectorize .COND_FMA you will need to add two arguments (as said, > >>no idea why we special case reduc_idx >= 0 at the moment). > > Do you mean I add gcc_unreachable in else like this: > > if (len_loop_p) > { > if (len_opno >= 0) > { > ifn = cond_len_fn; > /* COND_* -> COND_LEN_* takes 2 extra arguments:LEN,BIAS. */ > vect_nargs += 2; > } > else if (reduc_idx >= 0) > { > /* FMA -> COND_LEN_FMA takes 4 extra arguments:MASK,ELSE,LEN,BIAS. */ > ifn = get_len_internal_fn (cond_fn); > vect_nargs += 4; no, a gcc_unreachable () here. That is, make sure you have test coverage for the above two cases (to me the len_opno >= 0 case is obvious) > } > else > gcc_unreachable (); > } > > Thanks. > > > juzhe.zhong@rivai.ai > > From: Richard Biener > Date: 2023-07-31 21:58 > To: ??? > CC: richard.sandiford; gcc-patches > Subject: Re: Re: [PATCH V2] VECT: Support CALL vectorization for COND_LEN_* > On Mon, 31 Jul 2023, ??? wrote: > > > Yeah. I have tried this case too. > > > > But this case doesn't need to be vectorized as COND_FMA, am I right? > > Only when you enable loop masking. Alternatively use > > double foo (double *a, double *b, double *c) > { > double result = 0.0; > for (int i = 0; i < 1024; ++i) > result += i & 1 ? __builtin_fma (a[i], b[i], c[i]) : 0.0; > return result; > } > > but then for me if-conversion produces > > iftmp.0_18 = __builtin_fma (_8, _10, _5); > _ifc__43 = _26 ? iftmp.0_18 : 0.0; > > with -ffast-math (probably rightfully so). I then get .FMAs > vectorized and .COND_FMA folded. > > > The thing I wonder is that whether this condtion: > > > > if (mask_opno >= 0 && reduc_idx >= 0) > > > > or similar as len > > if (len_opno >= 0 && reduc_idx >= 0) > > > > Whether they are redundant in vectorizable_call ? > > > > > > juzhe.zhong@rivai.ai > > > > From: Richard Biener > > Date: 2023-07-31 21:33 > > To: juzhe.zhong@rivai.ai > > CC: richard.sandiford; gcc-patches > > Subject: Re: Re: [PATCH V2] VECT: Support CALL vectorization for COND_LEN_* > > On Mon, 31 Jul 2023, juzhe.zhong@rivai.ai wrote: > > > > > Hi, Richi. > > > > > > >> I think you need to use fma from math.h together with -ffast-math > > > >>to get fma. > > > > > > As you said, this is one of the case I tried: > > > https://godbolt.org/z/xMzrrv5dT > > > GCC failed to vectorize. > > > > > > Could you help me with this? > > > > double foo (double *a, double *b, double *c) > > { > > double result = 0.0; > > for (int i = 0; i < 1024; ++i) > > result += __builtin_fma (a[i], b[i], c[i]); > > return result; > > } > > > > with -mavx2 -mfma -Ofast this is vectorized on x86_64 to > > > > ... > > vect__9.13_27 = MEM [(double *)vectp_a.11_29]; > > _9 = *_8; > > vect__10.14_26 = .FMA (vect__7.10_30, vect__9.13_27, vect__4.7_33); > > vect_result_17.15_25 = vect__10.14_26 + vect_result_20.4_36; > > ... > > > > but ifcvt still shows > > > > _9 = *_8; > > _10 = __builtin_fma (_7, _9, _4); > > result_17 = _10 + result_20; > > > > still vectorizable_call has IFN_FMA with > > > > /* First try using an internal function. */ > > code_helper convert_code = MAX_TREE_CODES; > > if (cfn != CFN_LAST > > && (modifier == NONE > > || (modifier == NARROW > > && simple_integer_narrowing (vectype_out, vectype_in, > > &convert_code)))) > > ifn = vectorizable_internal_function (cfn, callee, vectype_out, > > vectype_in); > > > > from CFN_BUILT_IN_FMA > > > > > > > > > Thanks. > > > > > > > > > juzhe.zhong@rivai.ai > > > > > > From: Richard Biener > > > Date: 2023-07-31 20:00 > > > To: juzhe.zhong@rivai.ai > > > CC: richard.sandiford; gcc-patches > > > Subject: Re: Re: [PATCH V2] VECT: Support CALL vectorization for COND_LEN_* > > > On Mon, 31 Jul 2023, juzhe.zhong@rivai.ai wrote: > > > > > > > Ok . Thanks Richard. > > > > > > > > Could you give me a case that SVE can vectorize a reduction with FMA? > > > > Meaning it will go into vectorize_call and vectorize FMA into COND_FMA ? > > > > > > > > I tried many times to reproduce such cases but I failed. > > > > > > I think you need to use fma from math.h together with -ffast-math > > > to get fma. > > > > > > Richard. > > > > > > > Thanks. > > > > > > > > > > > > juzhe.zhong@rivai.ai > > > > > > > > From: Richard Sandiford > > > > Date: 2023-07-31 18:19 > > > > To: Juzhe-Zhong > > > > CC: gcc-patches; rguenther > > > > Subject: Re: [PATCH V2] VECT: Support CALL vectorization for COND_LEN_* > > > > Juzhe-Zhong writes: > > > > > Hi, Richard and Richi. > > > > > > > > > > Base on the suggestions from Richard: > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625396.html > > > > > > > > > > This patch choose (1) approach that Richard provided, meaning: > > > > > > > > > > RVV implements cond_* optabs as expanders. RVV therefore supports > > > > > both IFN_COND_ADD and IFN_COND_LEN_ADD. No dummy length arguments > > > > > are needed at the gimple level. > > > > > > > > > > Such approach can make codes much cleaner and reasonable. > > > > > > > > > > Consider this following case: > > > > > void foo (float * __restrict a, float * __restrict b, int * __restrict cond, int n) > > > > > { > > > > > for (int i = 0; i < n; i++) > > > > > if (cond[i]) > > > > > a[i] = b[i] + a[i]; > > > > > } > > > > > > > > > > > > > > > Output of RISC-V (32-bits) gcc (trunk) (Compiler #3) > > > > > :5:21: missed: couldn't vectorize loop > > > > > :5:21: missed: not vectorized: control flow in loop. > > > > > > > > > > ARM SVE: > > > > > > > > > > ... > > > > > mask__27.10_51 = vect__4.9_49 != { 0, ... }; > > > > > ... > > > > > vec_mask_and_55 = loop_mask_49 & mask__27.10_51; > > > > > ... > > > > > vect__9.17_62 = .COND_ADD (vec_mask_and_55, vect__6.13_56, vect__8.16_60, vect__6.13_56); > > > > > > > > > > For RVV, we want IR as follows: > > > > > > > > > > ... > > > > > _68 = .SELECT_VL (ivtmp_66, POLY_INT_CST [4, 4]); > > > > > ... > > > > > mask__27.10_51 = vect__4.9_49 != { 0, ... }; > > > > > ... > > > > > vect__9.17_60 = .COND_LEN_ADD (mask__27.10_51, vect__6.13_55, vect__8.16_59, vect__6.13_55, _68, 0); > > > > > ... > > > > > > > > > > Both len and mask of COND_LEN_ADD are real not dummy. > > > > > > > > > > This patch has been fully tested in RISC-V port with supporting both COND_* and COND_LEN_*. > > > > > > > > > > And also, Bootstrap and Regression on X86 passed. > > > > > > > > > > OK for trunk? > > > > > > > > > > gcc/ChangeLog: > > > > > > > > > > * internal-fn.cc (FOR_EACH_LEN_FN_PAIR): New macro. > > > > > (get_len_internal_fn): New function. > > > > > (CASE): Ditto. > > > > > * internal-fn.h (get_len_internal_fn): Ditto. > > > > > * tree-vect-stmts.cc (vectorizable_call): Support CALL vectorization with COND_LEN_*. > > > > > > > > > > --- > > > > > gcc/internal-fn.cc | 46 ++++++++++++++++++++++ > > > > > gcc/internal-fn.h | 1 + > > > > > gcc/tree-vect-stmts.cc | 87 +++++++++++++++++++++++++++++++++++++----- > > > > > 3 files changed, 125 insertions(+), 9 deletions(-) > > > > > > > > > > diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc > > > > > index 8e294286388..379220bebc7 100644 > > > > > --- a/gcc/internal-fn.cc > > > > > +++ b/gcc/internal-fn.cc > > > > > @@ -4443,6 +4443,52 @@ get_conditional_internal_fn (internal_fn fn) > > > > > } > > > > > } > > > > > > > > > > +/* Invoke T(IFN) for each internal function IFN that also has an > > > > > + IFN_COND_LEN_* or IFN_MASK_LEN_* form. */ > > > > > +#define FOR_EACH_LEN_FN_PAIR(T) \ > > > > > + T (MASK_LOAD, MASK_LEN_LOAD) \ > > > > > + T (MASK_STORE, MASK_LEN_STORE) \ > > > > > + T (MASK_GATHER_LOAD, MASK_LEN_GATHER_LOAD) \ > > > > > + T (MASK_SCATTER_STORE, MASK_LEN_SCATTER_STORE) \ > > > > > + T (COND_ADD, COND_LEN_ADD) \ > > > > > + T (COND_SUB, COND_LEN_SUB) \ > > > > > + T (COND_MUL, COND_LEN_MUL) \ > > > > > + T (COND_DIV, COND_LEN_DIV) \ > > > > > + T (COND_MOD, COND_LEN_MOD) \ > > > > > + T (COND_RDIV, COND_LEN_RDIV) \ > > > > > + T (COND_FMIN, COND_LEN_FMIN) \ > > > > > + T (COND_FMAX, COND_LEN_FMAX) \ > > > > > + T (COND_MIN, COND_LEN_MIN) \ > > > > > + T (COND_MAX, COND_LEN_MAX) \ > > > > > + T (COND_AND, COND_LEN_AND) \ > > > > > + T (COND_IOR, COND_LEN_IOR) \ > > > > > + T (COND_XOR, COND_LEN_XOR) \ > > > > > + T (COND_SHL, COND_LEN_SHL) \ > > > > > + T (COND_SHR, COND_LEN_SHR) \ > > > > > + T (COND_NEG, COND_LEN_NEG) \ > > > > > + T (COND_FMA, COND_LEN_FMA) \ > > > > > + T (COND_FMS, COND_LEN_FMS) \ > > > > > + T (COND_FNMA, COND_LEN_FNMA) \ > > > > > + T (COND_FNMS, COND_LEN_FNMS) > > > > > > > > With the earlier patch to add DEF_INTERNAL_COND_FN and > > > > DEF_INTERNAL_SIGNED_COND_FN, I think we should use those to handle > > > > the COND_* cases, rather than putting them in this macro. > > > > > > > > Thanks, > > > > Richard > > > > > > > > > > > > > > > > > > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)