From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id A629C3858C41 for ; Mon, 31 Jul 2023 05:44:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A629C3858C41 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com Received: from pps.filterd (m0353725.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 36V5cGEY016078; Mon, 31 Jul 2023 05:44:00 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pp1; bh=+etILBZ/NSjbxhP07B4BTrUVtLeQbzrkkqhku39wfFE=; b=GD3gf4Yu/QKbiSmH9JI9MjF+MOy8oxQJy7hJkx3Krs7SBiBjBEBkGi9VLyP6s4DOqYW4 Z4Ay8xVwJ10g3WFqAw7QIogvw2Fu5mJSpv1jbPQjhSMdSlbcvGABwpqFN6+MMqYqrnVZ vA/hEoDUlKQL44YXimNt7z+mfc/ks6OwBaPpPjEkePhTO3o6pFEunnidwGKRWICfT4zM 3mvcVsYDp9oZ5ogXnyuQHXGF3Tu9AdVAeWPlyapjGwvV0fD7h6k6NDLOms1miLVn09vr WWwdvUAmcQFW02xCjsPevHFFjxeKeg0+9bcjl58rtrKlFqgS6ZoLkb0RKfgcuWVgDA3D QA== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3s666g13fw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 31 Jul 2023 05:44:00 +0000 Received: from m0353725.ppops.net (m0353725.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 36V5cEAw015821; Mon, 31 Jul 2023 05:43:59 GMT Received: from ppma22.wdc07v.mail.ibm.com (5c.69.3da9.ip4.static.sl-reverse.com [169.61.105.92]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3s666g13fr-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 31 Jul 2023 05:43:59 +0000 Received: from pps.filterd (ppma22.wdc07v.mail.ibm.com [127.0.0.1]) by ppma22.wdc07v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 36V5NmNb017041; Mon, 31 Jul 2023 05:43:59 GMT Received: from smtprelay05.fra02v.mail.ibm.com ([9.218.2.225]) by ppma22.wdc07v.mail.ibm.com (PPS) with ESMTPS id 3s5dfxrtgc-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 31 Jul 2023 05:43:59 +0000 Received: from smtpav03.fra02v.mail.ibm.com (smtpav03.fra02v.mail.ibm.com [10.20.54.102]) by smtprelay05.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 36V5hu3Q16450080 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 31 Jul 2023 05:43:56 GMT Received: from smtpav03.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id EC0DC20043; Mon, 31 Jul 2023 05:43:55 +0000 (GMT) Received: from smtpav03.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id C490A20040; Mon, 31 Jul 2023 05:43:53 +0000 (GMT) Received: from [9.177.69.163] (unknown [9.177.69.163]) by smtpav03.fra02v.mail.ibm.com (Postfix) with ESMTP; Mon, 31 Jul 2023 05:43:53 +0000 (GMT) Message-ID: <3d2ac84a-be0c-d552-2c04-18a15a2d0278@linux.ibm.com> Date: Mon, 31 Jul 2023 13:43:51 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCHv2, rs6000] Generate mfvsrwz for all subtargets and remove redundant zero extend [PR106769] Content-Language: en-US To: HAO CHEN GUI Cc: Segher Boessenkool , David , Peter Bergner , gcc-patches References: <485dc9bf-fb4d-4a63-e9ec-0e58715d16da@linux.ibm.com> From: "Kewen.Lin" In-Reply-To: <485dc9bf-fb4d-4a63-e9ec-0e58715d16da@linux.ibm.com> Content-Type: text/plain; charset=UTF-8 X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: HP7kJep2xMEiRShl4r7VrV9b1sIfF5fD X-Proofpoint-GUID: I8utk2x95USdBrcHY_qQbb8bbSYMwVts Content-Transfer-Encoding: 7bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-07-27_10,2023-07-26_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 lowpriorityscore=0 suspectscore=0 bulkscore=0 spamscore=0 malwarescore=0 priorityscore=1501 impostorscore=0 phishscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2306200000 definitions=main-2307310049 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,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: Hi Haochen, on 2023/7/25 10:10, HAO CHEN GUI wrote: > Hi, > This patch modifies vsx extract expand and generates mfvsrwz/stxsiwx > for all subtargets when the mode is V4SI and the index of extracted element > is 1 for BE and 2 for LE. Also this patch adds a insn pattern for mfvsrwz > which helps eliminate redundant zero extend. > > Compared to last version, the main change is to move "vsx_extract_v4si_w1" > and "*mfvsrwz" to the front of "*vsx_extract__di_p9". Also some insn > conditions are changed to assertions. > https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625128.html Since the previous one is v2, this is actually v3. ;-) > > Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. > > Thanks > Gui Haochen > > ChangeLog > rs6000: Generate mfvsrwz for all platform and remove redundant zero extend > > mfvsrwz has lower latency than xxextractuw or vextuw[lr]x. So it should be > generated even with p9 vector enabled. Also the instruction is already > zero extended. A combine pattern is needed to eliminate redundant zero > extend instructions. > > gcc/ > PR target/106769 > * config/rs6000/vsx.md (expand vsx_extract_): Set it only > for V8HI and V16QI. > (vsx_extract_v4si): New expand for V4SI extraction. > (vsx_extract_v4si_w1): New insn pattern for V4SI extraction > when the index of extracted element is 1 with BE and 2 with LE. Nit: Maybe better to match the name with " ... for V4SI extraction on word 1 from BE order." > (*mfvsrwz): New insn pattern. > (*vsx_extract__di_p9): Not generate the insn when the index > of extracted element is 1 with BE and 2 with LE. > (*vsx_extract_si): Removed. Nit: s/Removed/Remove/ > (*vsx_extract_v4si_not_w1): New insn and split pattern which deals > with the cases not handled by vsx_extract_v4si_w1. > > gcc/testsuite/ > PR target/106769 > * gcc.target/powerpc/pr106769.h: New. > * gcc.target/powerpc/pr106769-p8.c: New. > * gcc.target/powerpc/pr106769-p9.c: New. > > patch.diff > diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md > index 0a34ceebeb5..0065b76fef8 100644 > --- a/gcc/config/rs6000/vsx.md > +++ b/gcc/config/rs6000/vsx.md > @@ -3722,9 +3722,9 @@ (define_insn "vsx_xxpermdi2__1" > (define_expand "vsx_extract_" > [(parallel [(set (match_operand: 0 "gpc_reg_operand") > (vec_select: > - (match_operand:VSX_EXTRACT_I 1 "gpc_reg_operand") > + (match_operand:VSX_EXTRACT_I2 1 "gpc_reg_operand") > (parallel [(match_operand:QI 2 "const_int_operand")]))) > - (clobber (match_scratch:VSX_EXTRACT_I 3))])] > + (clobber (match_scratch:VSX_EXTRACT_I2 3))])] > "VECTOR_MEM_VSX_P (mode) && TARGET_DIRECT_MOVE_64BIT" > { > /* If we have ISA 3.0, we can do a xxextractuw/vextractu{b,h}. */ > @@ -3736,6 +3736,63 @@ (define_expand "vsx_extract_" > } > }) > > +(define_expand "vsx_extract_v4si" > + [(parallel [(set (match_operand:SI 0 "gpc_reg_operand") > + (vec_select:SI > + (match_operand:V4SI 1 "gpc_reg_operand") > + (parallel [(match_operand:QI 2 "const_0_to_3_operand")]))) > + (clobber (match_scratch:V4SI 3))])] > + "TARGET_DIRECT_MOVE_64BIT" > +{ > + /* The word 1 (BE order) can be extracted by mfvsrwz/stxsiwx. So just > + fall through to vsx_extract_v4si_w1. */ > + if (TARGET_P9_VECTOR > + && INTVAL (operands[2]) != (BYTES_BIG_ENDIAN ? 1 : 2)) > + { > + emit_insn (gen_vsx_extract_v4si_p9 (operands[0], operands[1], > + operands[2])); > + DONE; > + } > +}) > + > +/* Extract from word 1 (BE order). */ Nit: Use semicolon ";" for comments to keep consistent with the others and what the doc says. > +(define_insn "vsx_extract_v4si_w1" > + [(set (match_operand:SI 0 "nonimmediate_operand" "=r,wa,Z,wa") > + (vec_select:SI > + (match_operand:V4SI 1 "gpc_reg_operand" "v,v,v,0") > + (parallel [(match_operand:QI 2 "const_0_to_3_operand" "n,n,n,n")]))) > + (clobber (match_scratch:V4SI 3 "=v,v,v,v"))] > + "TARGET_DIRECT_MOVE_64BIT > + && INTVAL (operands[2]) == (BYTES_BIG_ENDIAN ? 1 : 2)" > +{ > + if (which_alternative == 0) > + return "mfvsrwz %0,%x1"; > + > + if (which_alternative == 1) > + return "xxlor %x0,%x1,%x1"; > + > + if (which_alternative == 2) > + return "stxsiwx %x1,%y0"; > + > + return ASM_COMMENT_START " vec_extract to same register"; > +} > + [(set_attr "type" "mfvsr,veclogical,fpstore,*") > + (set_attr "length" "4,4,4,0") > + (set_attr "isa" "p8v,*,p8v,*")]) > + > +(define_insn "*mfvsrwz" > + [(set (match_operand:DI 0 "register_operand" "=r") > + (zero_extend:DI > + (vec_select:SI > + (match_operand:V4SI 1 "vsx_register_operand" "wa") > + (parallel [(match_operand:QI 2 "const_int_operand" "n")])))) > + (clobber (match_scratch:V4SI 3 "=v"))] > + "TARGET_DIRECT_MOVE_64BIT > + && INTVAL (operands[2]) == (BYTES_BIG_ENDIAN ? 1 : 2)" > + "mfvsrwz %0,%x1" > + [(set_attr "type" "mfvsr") > + (set_attr "isa" "p8v")]) > + > (define_insn "vsx_extract__p9" > [(set (match_operand: 0 "gpc_reg_operand" "=r,") > (vec_select: > @@ -3798,7 +3855,7 @@ (define_insn_and_split "*vsx_extract__di_p9" > (match_operand:VSX_EXTRACT_I 1 "gpc_reg_operand" "v,") > (parallel [(match_operand:QI 2 "const_int_operand" "n,n")])))) > (clobber (match_scratch:SI 3 "=r,X"))] > - "VECTOR_MEM_VSX_P (mode) && TARGET_VEXTRACTUB" > + "TARGET_VEXTRACTUB" Nit: I understand this change is based on TARGET_VEXTRACTUB implying the condition VECTOR_MEM_VSX_P (mode) always holds, but this line change isn't necessary in this patch and we have a few conditions like this, so I prefer to just leave this alone. > "#" > "&& reload_completed" > [(parallel [(set (match_dup 4) > @@ -3807,6 +3864,9 @@ (define_insn_and_split "*vsx_extract__di_p9" > (parallel [(match_dup 2)]))) > (clobber (match_dup 3))])] > { > + gcc_assert (mode != V4SImode > + || INTVAL (operands[2]) != (BYTES_BIG_ENDIAN ? 1 : 2)); > + > operands[4] = gen_rtx_REG (mode, REGNO (operands[0])); > } > [(set_attr "isa" "p9v,*")]) > @@ -3830,58 +3890,43 @@ (define_insn_and_split "*vsx_extract__store_p9" > (set (match_dup 0) > (match_dup 3))]) > > -(define_insn_and_split "*vsx_extract_si" > +/* Extract from word 0, 2, 3 (BE order). */ Nit: Use ";". > +(define_insn_and_split "*vsx_extract_v4si_not_w1" Nit: Maybe name it "...v4si_w023" to match the comment? > [(set (match_operand:SI 0 "nonimmediate_operand" "=r,wa,Z") > (vec_select:SI > (match_operand:V4SI 1 "gpc_reg_operand" "v,v,v") > (parallel [(match_operand:QI 2 "const_0_to_3_operand" "n,n,n")]))) > (clobber (match_scratch:V4SI 3 "=v,v,v"))] > - "VECTOR_MEM_VSX_P (V4SImode) && TARGET_DIRECT_MOVE_64BIT && !TARGET_P9_VECTOR" > + "TARGET_DIRECT_MOVE_64BIT > + && INTVAL (operands[2]) != (BYTES_BIG_ENDIAN ? 1 : 2)" I think the case "INTVAL (operands[2]) == (BYTES_BIG_ENDIAN ? 1 : 2)" should be already matched by vsx_extract_v4si_w1 above, so move this ... > "#" > - "&& reload_completed" > + "&& 1" > [(const_int 0)] > { > + gcc_assert (!TARGET_P9_VECTOR); ... here as an assertion too. > + > rtx dest = operands[0]; > rtx src = operands[1]; > rtx element = operands[2]; > - rtx vec_tmp = operands[3]; > - int value; > + rtx vec_tmp; > + > + if (GET_CODE (operands[3]) == SCRATCH) > + vec_tmp = gen_reg_rtx (V4SImode); > + else > + vec_tmp = operands[3]; > > /* Adjust index for LE element ordering, the below minuend 3 is computed by > GET_MODE_NUNITS (V4SImode) - 1. */ > if (!BYTES_BIG_ENDIAN) > element = GEN_INT (3 - INTVAL (element)); > > - /* If the value is in the correct position, we can avoid doing the VSPLT > - instruction. */ > - value = INTVAL (element); > - if (value != 1) > - emit_insn (gen_altivec_vspltw_direct (vec_tmp, src, element)); > - else > - vec_tmp = src; > - > - if (MEM_P (operands[0])) > - { > - if (can_create_pseudo_p ()) > - dest = rs6000_force_indexed_or_indirect_mem (dest); > - > - if (TARGET_P8_VECTOR) > - emit_move_insn (dest, gen_rtx_REG (SImode, REGNO (vec_tmp))); > - else > - emit_insn (gen_stfiwx (dest, gen_rtx_REG (DImode, REGNO (vec_tmp)))); > - } > + emit_insn (gen_altivec_vspltw_direct (vec_tmp, src, element)); > > - else if (TARGET_P8_VECTOR) > - emit_move_insn (dest, gen_rtx_REG (SImode, REGNO (vec_tmp))); > - else > - emit_move_insn (gen_rtx_REG (DImode, REGNO (dest)), > - gen_rtx_REG (DImode, REGNO (vec_tmp))); > + int value = BYTES_BIG_ENDIAN ? 1 : 2; > + emit_insn (gen_vsx_extract_v4si_w1 (dest, vec_tmp, GEN_INT (value))); > > DONE; > -} > - [(set_attr "type" "mfvsr,vecperm,fpstore") > - (set_attr "length" "8") > - (set_attr "isa" "*,p8v,*")]) > +}) > > (define_insn_and_split "*vsx_extract__p8" > [(set (match_operand: 0 "nonimmediate_operand" "=r") > diff --git a/gcc/testsuite/gcc.target/powerpc/pr106769-p8.c b/gcc/testsuite/gcc.target/powerpc/pr106769-p8.c > new file mode 100644 > index 00000000000..e7cdbc76298 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr106769-p8.c > @@ -0,0 +1,11 @@ > +/* { dg-do compile } */ > +/* { dg-skip-if "" { powerpc*-*-darwin* } } */ > +/* { dg-require-effective-target powerpc_p8vector_ok } */ > +/* { dg-options "-mdejagnu-cpu=power8 -O2" } */ > +/* { dg-require-effective-target has_arch_ppc64 } */ > + > +#include "pr106769.h" > + > +/* { dg-final { scan-assembler {\mmfvsrwz\M} } } */ > +/* { dg-final { scan-assembler {\mstxsiwx\M} } } */ > +/* { dg-final { scan-assembler-not {\mrldicl\M} } } */ > diff --git a/gcc/testsuite/gcc.target/powerpc/pr106769-p9.c b/gcc/testsuite/gcc.target/powerpc/pr106769-p9.c > new file mode 100644 > index 00000000000..2205e434a86 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr106769-p9.c > @@ -0,0 +1,13 @@ > +/* { dg-do compile } */ > +/* { dg-skip-if "" { powerpc*-*-darwin* } } */ > +/* { dg-require-effective-target powerpc_p9vector_ok } */ > +/* { dg-options "-mdejagnu-cpu=power9 -O2" } */ > +/* { dg-require-effective-target has_arch_ppc64 } */ > + > +#include "pr106769.h" > + > +/* { dg-final { scan-assembler {\mmfvsrwz\M} } } */ > +/* { dg-final { scan-assembler {\mstxsiwx\M} } } */ > +/* { dg-final { scan-assembler-not {\mrldicl\M} } } */ > +/* { dg-final { scan-assembler-not {\mxxextractuw\M} } } */ > +/* { dg-final { scan-assembler-not "vextuw\[rl\]x" } } */ Nit: I think the below with \m and \M should work: /* { dg-final { scan-assembler-not {\mvextuw[rl]x\M} } } */ Okay for trunk with all the nits above tweaked, thanks! BR, Kewen