Message ID | ZFu6I9FafoZHRYYi@toto.the-meissners.org |
---|---|
Headers |
Return-Path: <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp3718943vqo; Wed, 10 May 2023 08:39:34 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5TOjD1eQosMzCOG3DhOy6c5remEg7QO25BK0WfM+EIv+MHgUcle83Rb/4jTBSFeuJUgN3c X-Received: by 2002:a17:907:9718:b0:966:c09:1c57 with SMTP id jg24-20020a170907971800b009660c091c57mr15502642ejc.65.1683733174179; Wed, 10 May 2023 08:39:34 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683733174; cv=none; d=google.com; s=arc-20160816; b=m5Whb4sJChilOc+d7qyjORHlp7wGTyl/jNYJvVa4/ET303ne6IXKGDV/xungcwGTI7 bPJ/cBKJ94zqvRQWNGfSo6fc9YvnQBBMt2KmEw1+6i6ngNZkCXNgM4U1HmNZbZuvFmKg xkzFJGzsOhxLk5DzsMeP7v7FJnGBfNrGRxBrOyM0ctRFY3qyRuO1FsLM/uADxLcN55Dz 6Bm3UspubfMCFScgNCj2PepoZjUOgj7sU0oLoR8Xu0InMMZlcBX2QmnbF7toIXKZXqH5 SL/3Sk6JUIOKM+kzq/GFyPNo1oZTrsONqN0g/dYwjyE0/B9zn1YPE29VN6XljVHhmsoI 8zUg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-disposition:mime-version:mail-followup-to:message-id :subject:to:date:dmarc-filter:delivered-to:dkim-signature :dkim-filter; bh=Z7nyHLkT+jiK6xYqLMepa8+Rt4Nc+9BjXT4v+v4KdAM=; b=HyCEXZeBNrJ0r79uk4MYOc3L52eauWY3ik41xawPvejNMAPb/iOA9hrfLKjTRnCVqv gYCsglJ56j8PBhnTx9LY2tGAiJdwvPMDwWNh1pVlEVLUVdr56IsRUc6HrT0AF/9OmTQA AQsnTdZRM40/k8jSCLWyJrv3+tjqaNzHWjQdrJyKbkxtG/suhNg6IogcHgUKoweN9/DX E3/QmWWBQ76IiZWD+0y/R+3gWDS7+Q2clv/bhquJah5U1oMh/orXKNM2UAdRSO8cEJ8F p+Um4vPPiPTKSUoMLeGH+BVoev+7cpT4T/VTI7F0B2gCIn7sbEmGU8a4vKASvdYdw7i8 8QxQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=vRg0a5ZK; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id b9-20020a170906150900b00932af15caa7si2907556ejd.731.2023.05.10.08.39.33 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 May 2023 08:39:34 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=vRg0a5ZK; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 742CB384F022 for <ouuuleilei@gmail.com>; Wed, 10 May 2023 15:39:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 742CB384F022 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1683733171; bh=Z7nyHLkT+jiK6xYqLMepa8+Rt4Nc+9BjXT4v+v4KdAM=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=vRg0a5ZK6652s0n1ATiSlLPrW0F1Uryx8WteqQZFOvBJsctnum0JiW+T8j97cpX6d 9NeDojIbboYvhiRwCQCiTs0eaZsgIR4jU7ilAvIv2Oj4JxFkFx3qrBrTfL0Ke5GPEh 6zz4IA48GEjtqROKVT9wrh/2uy7y0yViEGDCFF/E= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id F3B57385558C for <gcc-patches@gcc.gnu.org>; Wed, 10 May 2023 15:38:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F3B57385558C Received: from pps.filterd (m0353729.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 34AFZgEl001186; Wed, 10 May 2023 15:38:47 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3qgaa2gayd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 10 May 2023 15:38:45 +0000 Received: from m0353729.ppops.net (m0353729.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 34AFZxwq003152; Wed, 10 May 2023 15:37:14 GMT Received: from ppma03dal.us.ibm.com (b.bd.3ea9.ip4.static.sl-reverse.com [169.62.189.11]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3qgaa2gavk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 10 May 2023 15:37:14 +0000 Received: from pps.filterd (ppma03dal.us.ibm.com [127.0.0.1]) by ppma03dal.us.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 34AE3253029893; Wed, 10 May 2023 15:37:10 GMT Received: from smtprelay02.dal12v.mail.ibm.com ([9.208.130.97]) by ppma03dal.us.ibm.com (PPS) with ESMTPS id 3qf7y93wtj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 10 May 2023 15:37:10 +0000 Received: from smtpav03.dal12v.mail.ibm.com (smtpav03.dal12v.mail.ibm.com [10.241.53.102]) by smtprelay02.dal12v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 34AFb9M019792516 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 10 May 2023 15:37:09 GMT Received: from smtpav03.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 63B425803F; Wed, 10 May 2023 15:37:09 +0000 (GMT) Received: from smtpav03.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E35B55805A; Wed, 10 May 2023 15:37:08 +0000 (GMT) Received: from toto.the-meissners.org (unknown [9.160.59.115]) by smtpav03.dal12v.mail.ibm.com (Postfix) with ESMTPS; Wed, 10 May 2023 15:37:08 +0000 (GMT) Date: Wed, 10 May 2023 11:37:07 -0400 To: gcc-patches@gcc.gnu.org, Michael Meissner <meissner@linux.ibm.com>, Segher Boessenkool <segher@kernel.crashing.org>, "Kewen.Lin" <linkw@linux.ibm.com>, David Edelsohn <dje.gcc@gmail.com>, Peter Bergner <bergner@linux.ibm.com> Subject: [PATCH V5, 0/2] PR target/105325: Fix constraint issue with power10 fusion Message-ID: <ZFu6I9FafoZHRYYi@toto.the-meissners.org> Mail-Followup-To: Michael Meissner <meissner@linux.ibm.com>, gcc-patches@gcc.gnu.org, Segher Boessenkool <segher@kernel.crashing.org>, "Kewen.Lin" <linkw@linux.ibm.com>, David Edelsohn <dje.gcc@gmail.com>, Peter Bergner <bergner@linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-TM-AS-GCONF: 00 X-Proofpoint-GUID: z9OIiAhMRbFlJS5S4kVzomb-mnkoqmEn X-Proofpoint-ORIG-GUID: TB8EqMvdSaS9_vr87GwEud55n4sjPzKS X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22 definitions=2023-05-10_04,2023-05-05_01,2023-02-09_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxlogscore=911 spamscore=0 impostorscore=0 malwarescore=0 lowpriorityscore=0 bulkscore=0 priorityscore=1501 phishscore=0 clxscore=1015 suspectscore=0 adultscore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000 definitions=main-2305100126 X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, RCVD_IN_MSPIKE_H2, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Michael Meissner via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Michael Meissner <meissner@linux.ibm.com> Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1765522196493329625?= X-GMAIL-MSGID: =?utf-8?q?1765522196493329625?= |
Series |
PR target/105325: Fix constraint issue with power10 fusion
|
|
Message
Michael Meissner
May 10, 2023, 3:37 p.m. UTC
I have posted 4 previous versions of this patch (April 26th, March 28th, March 24th, and March 21st). In this patch, rather than just add changes to the existing code in genfusion.pl, I rewrote the function completely. There are two patches within this patch set: * The first patch rewrites the perl function to be more readable. This patch produces the same output for fusion.md that the current version generates. * The second patch then using the rewrite in the first patch adds the changes to fix the problem. The issue with the original bug is the power10 load GPR + cmpi -1/0/1 fusion optimization generates illegal assembler code when the -fstack-protector option is used. Ultimately the code was dying because the fusion load + compare -1/0/1 patterns did not handle the possibility that the load might be prefixed. The main cause is the constraints for the individual loads in the fusion did not match the machine. In particular, LWA is a ds format instruction when it is unprefixed. The code did not also set the prefixed attribute correctly. These patch hav been tested on: * Little endian power9 with both IEEE and IBM long double * Little endian power10 * Big endian power8 using both 32-bit and 64-bit code generation. Can I check these into the master branch? Assuming I can check this in, I will also commit to the active GCC branches after a burn-in period.
Comments
This patch rewrites the gen_ld_cmpi_p10 function in genfusion.pl to be clearer. The resulting fusion.md file that this patch generates is exactly the same output that the previous version of genfusion.pl generated. The next patch in this series will fix PR target/105325 (provide correct predicates and constraints for power10 fusion of load and compare immediate). This patch has been tested on: * Little endian power9 with both IEEE and IBM long double * Little endian power10 * Big endian power8 using both 32-bit and 64-bit code generation. Can I check this into the master branch? Assuming I can check this in, I will also commit to the active GCC branches after a burn-in period. 2023-05-10 Michael Meissner <meissner@linux.ibm.com> gcc/ PR target/105325 * config/rs6000/genfusion.pl (mode_to_ldst_char): Delete. (print_ld_cmpi_p10): New function, split off from gen_ld_cmpi_p10. (gen_ld_cmpi_p10): Rewrite completely. --- gcc/config/rs6000/genfusion.pl | 248 +++++++++++++++++++++------------ 1 file changed, 157 insertions(+), 91 deletions(-) diff --git a/gcc/config/rs6000/genfusion.pl b/gcc/config/rs6000/genfusion.pl index e4db352e0ce..81ba4b33940 100755 --- a/gcc/config/rs6000/genfusion.pl +++ b/gcc/config/rs6000/genfusion.pl @@ -45,103 +45,169 @@ print <<'EOF'; EOF -sub mode_to_ldst_char +# Print the insns for load and compare with -1/0/1. +# Arguments: +# lmode -- Integer mode ("DI", "SI", "HI", or "QI"). +# result -- "clobber", "GPR", or $lmode +# ccmode -- Sign vs. unsigned ("CC" or "CCUNS"). +# mem_format -- Memory format ("d" or "ds"). +# cmpl -- Suffix for compare ("l" or "") +# const_pred -- Predicate for constant (i.e. -1/0/1 or 0/1). +# extend -- "sign", "zero", or "none". +# echr -- Suffix for load ("a", "z", or ""). +# load -- Load instruction (i.e. "ld", "lwa", "lwz", etc.) +# np -- enum non_prefixed_form for memory type +# constraint -- constraint to use +# mem_pred -- predicate for the memory operation + +sub print_ld_cmpi_p10 { - my ($mode) = @_; - my %x = (DI => 'd', SI => 'w', HI => 'h', QI => 'b'); - return $x{$mode} if exists $x{$mode}; - return '?'; + my ($lmode, $result, $ccmode, $cmpl, $const_pred, + $extend, $load, $np, $constraint, $mem_pred) = @_; + + # For clobber, we need a SI/DI reg in case we split because we have to + # sign/zero extend. + my $clobbermode = ($lmode =~ m/^[HQ]I$/) ? "GPR" : $lmode; + + # Break long print statements into smaller lines. + my $info = join (" ", + "load mode is ${lmode} result mode is ${result}", + "compare mode is ${ccmode} extend is ${extend}"); + + my $name = join ("", + "${load}_cmp${cmpl}di_cr0_${lmode}", + "_${result}_${ccmode}_${extend}"); + + my $cmp_op1 = "(match_operand:${lmode} 1 \"${mem_pred}\" \"${constraint}\")"; + + my $spaces = " " x (length ($ccmode) + 18); + + print ";; load-cmpi fusion pattern generated by gen_ld_cmpi_p10\n"; + print ";; ${info}\n"; + print "(define_insn_and_split \"*${name}\"\n"; + print " [(set (match_operand:${ccmode} 2 \"cc_reg_operand\" \"=x\")\n"; + print " (compare:${ccmode} ${cmp_op1}\n"; + print "${spaces}(match_operand:${lmode} 3 \"${const_pred}\" \"n\")))\n"; + + if ($result eq "clobber") + { + print " (clobber (match_scratch:${clobbermode} 0 \"=r\"))]\n"; + } + + else + { + my $load_op0 = "(match_operand:${result} 0 \"gpc_reg_operand\" \"=r\")"; + my $load_op1 = (($result eq $lmode) + ? "(match_dup 1)" + : "(${extend}_extend:${result} (match_dup 1))"); + print " (set ${load_op0} ${load_op1})]\n"; + } + + # Do not match prefixed loads. The machine only fuses non-prefixed loads + # with compare immediate. Take into account whether the load is a ds-form + # or a d-form instruction. + print " \"(TARGET_P10_FUSION)\"\n"; + print " \"${load}%X1 %0,%1\\;cmp${cmpl}di %2,%0,%3\"\n"; + print " \"&& reload_completed\n"; + print " && (cc_reg_not_cr0_operand (operands[2], CCmode)\n"; + print " || !address_is_non_pfx_d_or_x (XEXP (operands[1], 0),\n"; + print " ${lmode}mode, ${np}))\"\n"; + + if ($extend eq "none") + { + print " [(set (match_dup 0) (match_dup 1))\n"; + } + + else + { + my $resultmode = ($result eq "clobber") ? $clobbermode : $result; + print " [(set (match_dup 0) (${extend}_extend:${resultmode} (match_dup 1)))\n"; + } + + print " (set (match_dup 2)\n"; + print " (compare:${ccmode} (match_dup 0) (match_dup 3)))]\n"; + print " \"\"\n"; + print " [(set_attr \"type\" \"fused_load_cmpi\")\n"; + print " (set_attr \"cost\" \"8\")\n"; + print " (set_attr \"length\" \"8\")])\n"; + print "\n"; } sub gen_ld_cmpi_p10 { - my ($lmode, $ldst, $clobbermode, $result, $cmpl, $echr, $constpred, - $mempred, $ccmode, $np, $extend, $resultmode); - LMODE: foreach $lmode ('DI','SI','HI','QI') { - $ldst = mode_to_ldst_char($lmode); - $clobbermode = $lmode; - # For clobber, we need a SI/DI reg in case we - # split because we have to sign/zero extend. - if ($lmode eq 'HI' || $lmode eq 'QI') { $clobbermode = "GPR"; } - RESULT: foreach $result ('clobber', $lmode, "EXT".$lmode) { - # EXTDI does not exist, and we cannot directly produce HI/QI results. - next RESULT if $result eq "EXTDI" || $result eq "HI" || $result eq "QI"; - # Don't allow EXTQI because that would allow HI result which we can't do. - $result = "GPR" if $result eq "EXTQI"; - CCMODE: foreach $ccmode ('CC','CCUNS') { - $np = "NON_PREFIXED_D"; - $mempred = "non_update_memory_operand"; - if ( $ccmode eq 'CC' ) { - next CCMODE if $lmode eq 'QI'; - if ( $lmode eq 'DI' || $lmode eq 'SI' ) { - # ld and lwa are both DS-FORM. - $np = "NON_PREFIXED_DS"; - $mempred = "ds_form_mem_operand"; - } - $cmpl = ""; - $echr = "a"; - $constpred = "const_m1_to_1_operand"; - } else { - if ( $lmode eq 'DI' ) { - # ld is DS-form, but lwz is not. - $np = "NON_PREFIXED_DS"; - $mempred = "ds_form_mem_operand"; - } - $cmpl = "l"; - $echr = "z"; - $constpred = "const_0_to_1_operand"; - } - if ($lmode eq 'DI') { $echr = ""; } - if ($result =~ m/^EXT/ || $result eq 'GPR' || $clobbermode eq 'GPR') { - # We always need extension if result > lmode. - if ( $ccmode eq 'CC' ) { - $extend = "sign"; - } else { - $extend = "zero"; - } - } else { - # Result of SI/DI does not need sign extension. - $extend = "none"; - } - print ";; load-cmpi fusion pattern generated by gen_ld_cmpi_p10\n"; - print ";; load mode is $lmode result mode is $result compare mode is $ccmode extend is $extend\n"; - - print "(define_insn_and_split \"*l${ldst}${echr}_cmp${cmpl}di_cr0_${lmode}_${result}_${ccmode}_${extend}\"\n"; - print " [(set (match_operand:${ccmode} 2 \"cc_reg_operand\" \"=x\")\n"; - print " (compare:${ccmode} (match_operand:${lmode} 1 \"${mempred}\" \"m\")\n"; - if ($ccmode eq 'CCUNS') { print " "; } - print " (match_operand:${lmode} 3 \"${constpred}\" \"n\")))\n"; - if ($result eq 'clobber') { - print " (clobber (match_scratch:${clobbermode} 0 \"=r\"))]\n"; - } elsif ($result eq $lmode) { - print " (set (match_operand:${result} 0 \"gpc_reg_operand\" \"=r\") (match_dup 1))]\n"; - } else { - print " (set (match_operand:${result} 0 \"gpc_reg_operand\" \"=r\") (${extend}_extend:${result} (match_dup 1)))]\n"; - } - print " \"(TARGET_P10_FUSION)\"\n"; - print " \"l${ldst}${echr}%X1 %0,%1\\;cmp${cmpl}di %2,%0,%3\"\n"; - print " \"&& reload_completed\n"; - print " && (cc_reg_not_cr0_operand (operands[2], CCmode)\n"; - print " || !address_is_non_pfx_d_or_x (XEXP (operands[1], 0),\n"; - print " ${lmode}mode, ${np}))\"\n"; - - if ($extend eq "none") { - print " [(set (match_dup 0) (match_dup 1))\n"; - } else { - $resultmode = $result; - if ( $result eq 'clobber' ) { $resultmode = $clobbermode } - print " [(set (match_dup 0) (${extend}_extend:${resultmode} (match_dup 1)))\n"; - } - print " (set (match_dup 2)\n"; - print " (compare:${ccmode} (match_dup 0) (match_dup 3)))]\n"; - print " \"\"\n"; - print " [(set_attr \"type\" \"fused_load_cmpi\")\n"; - print " (set_attr \"cost\" \"8\")\n"; - print " (set_attr \"length\" \"8\")])\n"; - print "\n"; - } + my ($lmode, $result, $mem_format, $extend); + + # Map mode to load instruction + my %signed_load = ("DI" => "ld", + "SI" => "lwa", + "HI" => "lha"); + + my %unsigned_load = ("DI" => "ld", + "SI" => "lwz", + "HI" => "lhz", + "QI" => "lbz"); + + # Memory predicate to use. + my %signed_memory_predicate = ("DI" => "ds_form_mem_operand", + "SI" => "ds_form_mem_operand", + "HI" => "non_update_memory_operand"); + + my %unsigned_memory_predicate = ("DI" => "ds_form_mem_operand", + "SI" => "non_update_memory_operand", + "HI" => "non_update_memory_operand", + "QI" => "non_update_memory_operand"); + + # Internal format of the memory instruction (enum non_prefixed_form) to use. + my %np = ("ds" => "NON_PREFIXED_DS", + "d" => "NON_PREFIXED_D"); + + # Result modes to use. Clobber is used when you are comparing the load to + # -1/0/1, but you are not using it otherwise. EXTDI does not exist. We + # cannot directly use HI/QI results because we only have word and double word + # compared. For promotion, don't allow EXTQI because that would allow HI + # results which we can't do (use GPR instead). + my %result_modes = ("DI" => ["clobber", "DI"], + "SI" => ["clobber", "SI", "EXTSI" ], + "HI" => ["clobber", "EXTHI" ], + "QI" => ["clobber", "GPR" ]); + + foreach $lmode ("DI", "SI", "HI", "QI") + { + foreach $result (@{ $result_modes{$lmode} }) + { + # Handle CCmode (sign extended compares to -1, 0, or 1). We don't + # have a LBA instruction, so skip QImode. Both LD and LWA are + # DS-form instructions for signed loads. + if ($lmode ne "QI") + { + $mem_format = ($lmode =~ m/^[DS]I$/) ? "ds" : "d"; + $extend = (($lmode eq "DI" + || $lmode eq $result + || ($lmode eq "SI" && $result eq "clobber")) + ? "none" + : "sign"); + + print_ld_cmpi_p10 ($lmode, $result, "CC", "", + "const_m1_to_1_operand", $extend, + $signed_load{$lmode}, $np{$mem_format}, "m", + $signed_memory_predicate{$lmode}); + } + + # Handle CCUNS mode (zero extended compares to 0 or 1. + # LD is DS-form, but LWZ is not for unsigned loads. + $mem_format = ($lmode eq "DI") ? "ds" : "d"; + $extend = (($lmode eq "DI" + || $lmode eq $result + || ($lmode eq "SI" && $result eq "clobber")) + ? "none" + : "zero"); + + print_ld_cmpi_p10 ($lmode, $result, "CCUNS", "l", + "const_0_to_1_operand", $extend, + $unsigned_load{$lmode}, $np{$mem_format}, "m", + $unsigned_memory_predicate{$lmode}); + } } - } } sub gen_logical_addsubf
Ping both patches: Patch #1, rewrite genfusion.pl's code for load and compare immediate fusion to be more readable. This patch produces the same output as the current sources. | Date: Wed, 10 May 2023 11:38:55 -0400 | Subject: Re: [PATCH V5, 1/2] PR target/105325: Rewrite genfusion.pl's gen_ld_cmpi_p10 function. | Message-ID: <ZFu6j7SZaCUZpTYb@toto.the-meissners.org> Patch #2, implement the fix for PR target/105325: | Date: Wed, 10 May 2023 11:40:00 -0400 | Subject: [PATCH V5, 2/2] PR target/105325: Fix memory constraints for power10 fusion. | Message-ID: <ZFu60EPEOJTV/GA1@toto.the-meissners.org>
Hi Mike, On Wed, May 10, 2023 at 11:38:55AM -0400, Michael Meissner wrote: > This patch rewrites the gen_ld_cmpi_p10 function in genfusion.pl to be clearer. That is not at all what I asked for, even if I would agree the code is nicer to read now (I don't). What I asked for, what is needed, is for your patches to be readable. This is a prerequisite for them to be reviewable, which is a prerequisite for them to be approvable. One way to do that is to split out refactorings (which I asked for) and rewrites (which you did) to earlier patches in the series. Pure refactoring are easy to review: they change exactly nothing in what code is executed. Rewrites are much harder to review. But even then we can hope you didn't slip up once in a hundred lines of code, sure. The later patches can then be much more readable because there isn't so much noise mixed in. > Assuming I can check this in, I will > also commit to the active GCC branches after a burn-in period. No, you will never do that. You always need approval for that. We have these procedures for a reason. We do not want other things than what was approved committed, doubly so if *nothing* was approved. > * config/rs6000/genfusion.pl (mode_to_ldst_char): Delete. This is a regression. > +# Print the insns for load and compare with -1/0/1. > +# Arguments: > +# lmode -- Integer mode ("DI", "SI", "HI", or "QI"). > +# result -- "clobber", "GPR", or $lmode > +# ccmode -- Sign vs. unsigned ("CC" or "CCUNS"). > +# mem_format -- Memory format ("d" or "ds"). > +# cmpl -- Suffix for compare ("l" or "") > +# const_pred -- Predicate for constant (i.e. -1/0/1 or 0/1). > +# extend -- "sign", "zero", or "none". > +# echr -- Suffix for load ("a", "z", or ""). > +# load -- Load instruction (i.e. "ld", "lwa", "lwz", etc.) > +# np -- enum non_prefixed_form for memory type > +# constraint -- constraint to use > +# mem_pred -- predicate for the memory operation If you need a huge block comment for your sub argument, that is a not-so-subtle hint that you need to refactor. Or if this was supposed to be a refactoring, that something went terribly wrong :-( Segher