From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 49B5F3858D1E for ; Wed, 21 Dec 2022 03:42:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 49B5F3858D1E 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 (m0098404.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 2BL3faCs026177; Wed, 21 Dec 2022 03:42:09 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=m87/x2PEBVUGp8KuNtqSJXhKteRR6QOsgotBCUxhIP4=; b=rJsGjU6ptbBUVEjhaZVN+eVvR4NktpNx0jLEVt3Xm2ejhGjkPRqHrn2BLFxSQ/1jmBYH wYr73CqwHF048HvuDV8bT6iOpdyaQMqqvN8xfGxzB6gjXRETaCeVABYKPnvorJoSpu4p t+qZxsPDRsJEUnE4j/0UBeX6TXQB/xByCALBnrTHEn8OHb1cOFS3kUkNhzj+4i6FGHDO qJ263XHGrnAba+0HKdn1z/M9ulD+elFAiiKFO0owGfEYJJsCLUBMk1vxv1lNfmUzPUka ejI+iM/yfsf0i/4Hfuo9Bskaj0yubABiqWg3MrMJD8WQDXLI69Zwlz/6Ioem9Gvgj1Mc 7g== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3mktf6r0gs-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 21 Dec 2022 03:42:09 +0000 Received: from m0098404.ppops.net (m0098404.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 2BL3g8PS032510; Wed, 21 Dec 2022 03:42:08 GMT Received: from ppma06ams.nl.ibm.com (66.31.33a9.ip4.static.sl-reverse.com [169.51.49.102]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3mktf6r0fv-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 21 Dec 2022 03:42:08 +0000 Received: from pps.filterd (ppma06ams.nl.ibm.com [127.0.0.1]) by ppma06ams.nl.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 2BKJSfSN020557; Wed, 21 Dec 2022 03:42:06 GMT Received: from smtprelay03.fra02v.mail.ibm.com ([9.218.2.224]) by ppma06ams.nl.ibm.com (PPS) with ESMTPS id 3mh6yy4vu4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 21 Dec 2022 03:42:06 +0000 Received: from smtpav04.fra02v.mail.ibm.com (smtpav04.fra02v.mail.ibm.com [10.20.54.103]) by smtprelay03.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 2BL3g2qx46465496 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 21 Dec 2022 03:42:02 GMT Received: from smtpav04.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 8936F20049; Wed, 21 Dec 2022 03:42:02 +0000 (GMT) Received: from smtpav04.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7CDEA20040; Wed, 21 Dec 2022 03:42:00 +0000 (GMT) Received: from [9.197.239.17] (unknown [9.197.239.17]) by smtpav04.fra02v.mail.ibm.com (Postfix) with ESMTP; Wed, 21 Dec 2022 03:42:00 +0000 (GMT) Message-ID: <87345a42-055d-a104-bf43-7721e4b3bc9f@linux.ibm.com> Date: Wed, 21 Dec 2022 11:41:58 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH] rs6000: Fix some issues related to Power10 fusion [PR104024] Content-Language: en-US To: Segher Boessenkool Cc: GCC Patches , Peter Bergner , Michael Meissner , David Edelsohn References: <009fda27-7119-6de8-8dbe-51126bdfca12@linux.ibm.com> <20221214222944.GR25951@gate.crashing.org> <20221220131904.GR25951@gate.crashing.org> From: "Kewen.Lin" In-Reply-To: <20221220131904.GR25951@gate.crashing.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: fKiPCUFthNrsnR_ZLyaS-QDV56G-7dxs X-Proofpoint-ORIG-GUID: yWFGJpJw367UQgAF2kQ6t4gWT4_jjs4k X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-12-20_08,2022-12-20_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 phishscore=0 priorityscore=1501 bulkscore=0 adultscore=0 spamscore=0 malwarescore=0 mlxlogscore=999 lowpriorityscore=0 suspectscore=0 impostorscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2212210022 X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_MSPIKE_H2,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 Segher, on 2022/12/20 21:19, Segher Boessenkool wrote: > Hi! > > On Mon, Dec 19, 2022 at 02:13:49PM +0800, Kewen.Lin wrote: >> on 2022/12/15 06:29, Segher Boessenkool wrote: >>> On Wed, Nov 30, 2022 at 04:30:13PM +0800, Kewen.Lin wrote: >>>> --- a/gcc/config/rs6000/genfusion.pl >>>> +++ b/gcc/config/rs6000/genfusion.pl >>>> @@ -167,7 +167,7 @@ sub gen_logical_addsubf >>>> $inner_comp, $inner_inv, $inner_rtl, $inner_op, $both_commute, $c4, >>>> $bc, $inner_arg0, $inner_arg1, $inner_exp, $outer_arg2, $outer_exp, >>>> $ftype, $insn, $is_subf, $is_rsubf, $outer_32, $outer_42,$outer_name, >>>> - $fuse_type); >>>> + $fuse_type, $constraint_cond); >>>> KIND: foreach $kind ('scalar','vector') { >>>> @outer_ops = @logicals; >>>> if ( $kind eq 'vector' ) { >>>> @@ -176,12 +176,14 @@ sub gen_logical_addsubf >>>> $pred = "altivec_register_operand"; >>>> $constraint = "v"; >>>> $fuse_type = "fused_vector"; >>>> + $constraint_cond = "VECTOR_UNIT_ALTIVEC_OR_VSX_P (mode) && "; >>>> } else { >>>> $vchr = ""; >>>> $mode = "GPR"; >>>> $pred = "gpc_reg_operand"; >>>> $constraint = "r"; >>>> $fuse_type = "fused_arith_logical"; >>>> + $constraint_cond = ""; >>>> push (@outer_ops, @addsub); >>>> push (@outer_ops, ( "rsubf" )); >>>> } >>> >>> I don't like this at all. Please use the "isa" attribute where needed? >>> Or do you need more in some cases? But, again, separate patch. >> >> This is to add one more condition for those define_insns, for example: > > Sure, I understand that. What I don't like is the generator program is > much too big and unstructured already, and this doesn't help at all; it > makes it quite a bit worse even. OK. > >> It's to avoid the pseudo whose mode isn't available for register constraint v >> causes ICE during reload. I'm not sure how the "isa" attribute helps here, >> could you elaborate it? > > Yeah, it doesn't help. The condition implied by the isa attribute is > not added to the insn condition automatically; doing that could be too > expensive, and disruptive as well. Something for stage 1 :-) > OK, thanks for the clarification. :) >>>> + if (TARGET_POWER10 >>>> + && (rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION) == 0) >>>> + rs6000_isa_flags |= OPTION_MASK_P10_FUSION; >>>> + else if (!TARGET_POWER10 && TARGET_P10_FUSION) >>>> + rs6000_isa_flags &= ~OPTION_MASK_P10_FUSION; >>> >>> That's not right. If you want something like this you should check for >>> TARGET_POWER10 whenever you check for TARGET_P10_FUSION; but there >>> really is no reason at all to disable P10 fusion on other CPUs (neither >>> newer nor older!). >> >> Good point, and I just noticed that we should check tune setting instead >> of TARGET_POWER10 here? Something like: >> >> if (!(rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION)) >> { >> if (processor_target_table[tune_index].processor == PROCESSOR_POWER10) >> rs6000_isa_flags |= OPTION_MASK_P10_FUSION; >> else >> rs6000_isa_flags &= ~OPTION_MASK_P10_FUSION; >> } > > Yeah that looks better :-) > I'm going to test this and commit it first. :) > Maybe you can restructure the Perl code a bit in a first patch, and then > add the insn condition? If you're not comfortable with Perl, I'll deal > with it, just update the patch. OK, I'll give it a try, TBH I just fixed the place for insn condition, didn't look into this script, with a quick look, I'm going to factor out the main body from the multiple level loop, do you have some suggestions on which other candidates to be restructured? BR, Kewen