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 88FC33858D37 for ; Thu, 20 Apr 2023 21:21:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 88FC33858D37 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 (m0353728.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 33KLBu0C020206; Thu, 20 Apr 2023 21:21:12 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : subject : from : to : cc : references : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=3065Hq3lEfWdxYLn4mkbWjdD1heq78IqWoTM1nD1Wl0=; b=PaPgxNO9nfoCQqqq3nbMxJGUI5amPdQgsvAjjXclSG388tbjZ1UEW3pxknN6smzfJu15 W9exOq/WloygDCfy6/74jLDKAeeSP+dhnnCOO0gHJt2GS2PSyMOs744MFOmD6Lz2qPaT Rl/G1JaHJze0ReCxNxX36YtJmfxl8+buPNtAoKQJISzs45xbcU55ZM5cOSTmXoDFcGlt pWOodOHeSQ9zCLjs+BmHL91ZQfNsZvp1ONusisWPQEoJzD61fET562UtNZHhNsbVYr6F bqo3O7mx9+2FtqFQJZLDVWwU6uiuy0Weq4830b6dM+rdB2G3B1g4HwY3Kvw0JCvnyjv4 9g== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3q3d3hga05-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 20 Apr 2023 21:21:12 +0000 Received: from m0353728.ppops.net (m0353728.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 33KLDNLd026518; Thu, 20 Apr 2023 21:21:11 GMT Received: from ppma02dal.us.ibm.com (a.bd.3ea9.ip4.static.sl-reverse.com [169.62.189.10]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3q3d3hg9y7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 20 Apr 2023 21:21:11 +0000 Received: from pps.filterd (ppma02dal.us.ibm.com [127.0.0.1]) by ppma02dal.us.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 33KJUIYP026562; Thu, 20 Apr 2023 21:21:10 GMT Received: from smtprelay06.wdc07v.mail.ibm.com ([9.208.129.118]) by ppma02dal.us.ibm.com (PPS) with ESMTPS id 3q1uxdrcx8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 20 Apr 2023 21:21:10 +0000 Received: from smtpav01.wdc07v.mail.ibm.com (smtpav01.wdc07v.mail.ibm.com [10.39.53.228]) by smtprelay06.wdc07v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 33KLL8YC6161084 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 20 Apr 2023 21:21:09 GMT Received: from smtpav01.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id C356C5804B; Thu, 20 Apr 2023 21:21:08 +0000 (GMT) Received: from smtpav01.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3603158055; Thu, 20 Apr 2023 21:21:06 +0000 (GMT) Received: from [9.43.43.199] (unknown [9.43.43.199]) by smtpav01.wdc07v.mail.ibm.com (Postfix) with ESMTP; Thu, 20 Apr 2023 21:21:05 +0000 (GMT) Message-ID: Date: Fri, 21 Apr 2023 02:51:04 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Subject: Re: [PATCH v3 3/4] ree: Main functionality to Improve ree pass for rs6000 target Content-Language: en-US From: Ajit Agarwal To: Jeff Law , gcc-patches Cc: Segher Boessenkool , Peter Bergner , Jakub Jelinek , Richard Biener References: <12889922-0160-a036-7dbf-1d208e353f82@linux.ibm.com> <7834f9e4-6ea3-8131-72ad-be6b37e5432f@gmail.com> <760bf382-856c-e6cd-25ec-9a3086b1a344@linux.ibm.com> In-Reply-To: <760bf382-856c-e6cd-25ec-9a3086b1a344@linux.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: __3DBP-RTazTOwrRSVjxqqOTGtZ6lnjZ X-Proofpoint-ORIG-GUID: Fygh8uXSma2fr_ycusTwfjZFkiq0encP 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-04-20_15,2023-04-20_01,2023-02-09_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 mlxlogscore=999 clxscore=1015 spamscore=0 suspectscore=0 mlxscore=0 malwarescore=0 bulkscore=0 priorityscore=1501 phishscore=0 lowpriorityscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2303200000 definitions=main-2304200177 X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,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: Hello Jeff: On 21/04/23 2:33 am, Ajit Agarwal wrote: > Hello Jeff: > > On 20/04/23 3:23 am, Jeff Law wrote: >> >> >> On 4/19/23 12:00, Ajit Agarwal wrote: >>> Hello All: >>> >>> This is patch-3 to improve ree pass for rs6000 target. >>> Main functionality routines to imprve ree pass. >>> >>> Bootstrapped and regtested on powerpc64-gnu-linux. >>> >>> Thanks & Regards >>> Ajit >>> >>>     ree: Improve ree pass for rs6000 target. >>> >>>     For rs6000 target we see redundant zero and sign >>>     extension and done to improve ree pass to eliminate >>>     such redundant zero and sign extension. Support of >>>     zero_extend/sign_extend/AND. >>> >>>     2023-04-19  Ajit Kumar Agarwal  >>> >>> gcc/ChangeLog: >>> >>>     * ree.cc (eliminate_across_bbs_p): Add checks to enable extension >>>     elimination across and within basic blocks. >>>     (def_arith_p): New function to check definition has arithmetic >>>     operation. >>>     (combine_set_extension): Modification to incorporate AND >>>     and current zero_extend and sign_extend instruction. >>>     (merge_def_and_ext): Add calls to eliminate_across_bbs_p and >>>     zero_extend sign_extend and AND instruction. >>>     (rtx_is_zext_p): New function. >>>     (reg_used_set_between_p): New function. >>> >>> gcc/testsuite/ChangeLog: >>> >>>     * g++.target/powerpc/zext-elim.C: New testcase. >>>     * g++.target/powerpc/zext-elim-1.C: New testcase. >>>     * g++.target/powerpc/zext-elim-2.C: New testcase. >>>     * g++.target/powerpc/sext-elim.C: New testcase. >>> --- >>>   gcc/ree.cc                                    | 451 ++++++++++++++++-- >>>   gcc/testsuite/g++.target/powerpc/sext-elim.C  |  18 + >>>   .../g++.target/powerpc/zext-elim-1.C          |  19 + >>>   .../g++.target/powerpc/zext-elim-2.C          |  11 + >>>   gcc/testsuite/g++.target/powerpc/zext-elim.C  |  30 ++ >>>   5 files changed, 482 insertions(+), 47 deletions(-) >>>   create mode 100644 gcc/testsuite/g++.target/powerpc/sext-elim.C >>>   create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-1.C >>>   create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-2.C >>>   create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim.C >>> >>> diff --git a/gcc/ree.cc b/gcc/ree.cc >>> index 413aec7c8eb..053db2e8ff3 100644 >>> --- a/gcc/ree.cc >>> +++ b/gcc/ree.cc >>> @@ -253,6 +253,71 @@ struct ext_cand >>>     static int max_insn_uid; >>>   +bool >>> +reg_used_set_between_p (rtx set, rtx_insn *def_insn, rtx_insn *insn >>> +{ >>> +  if (reg_used_between_p (set, def_insn, insn) >>> +      || reg_set_between_p (set, def_insn, insn)) >>> +    return true; >>> + >>> +  return false; >>> +} >> This seems general enough that it should go into the same file as reg_used_between_p and reg_set_between_p.  It needs a function comment as well. >> >> >>> +static unsigned int >>> +rtx_is_zext_p (rtx insn) >>> +{ >>> +  if (GET_CODE (insn) == AND) >>> +    { >>> +      rtx set = XEXP (insn, 0); >>> +      if (REG_P (set)) >>> +    { >>> +      if (XEXP (insn, 1) == const1_rtx) >>> +        return 1; >>> +    } >>> +      else >>> +    return 0; >>> +    } >>> + >>> +  return 0; >>> +} >> So my comment from the prior version stands.  Testing for const1_rtx is just wrong.  The optimization you're trying to perform (If I understand it correctly) works for many other constants and the set of constants supported will vary based on the input and output modes. >> >> Similarly in rtx_is_zext_p. >> >> You still have numerous formatting issues which makes reading the patch more difficult than it should be.  Please review the formatting guidelines and follow them.   In particular please review how to indent multi-line conditionals. >> >> > > Currently I support AND with const1_rtx. This is what is equivalent to zero extension instruction in power instruction set. When you specify many other constants and Could you please specify what other constants needs to be supported and how to determine on the Input and output modes. On top of that I support eliminating zero_extend and sign_extend wherein if result mode of def insn not equal to source operand of zero_extend and sign_extend. Thanks & Regards Ajit >> >> >> >> You sti >>> @@ -698,6 +777,226 @@ get_sub_rtx (rtx_insn *def_insn) >>>     return sub_rtx; >>>   } >>>   +/* Check if the def insn is ASHIFT and LSHIFTRT. >>> +  Inputs: insn for which def has to be checked. >>> +      source operand rtx. >>> +   Output: True or false if def has arithmetic >>> +       peration like ASHIFT and LSHIFTRT.  */ >> This still needs work.  Between the comments and code, I still don't know what you're really trying to do here.  I can make some guesses, but it's really your job to write clear comments about what you're doing so that a review or someone looking at the code in the future don't have to guess. >> >> It looks like you want to look at all the reaching definitions of INSN for ORIG_SRC and if they are ASHIFT/LSHIFTRT do...  what? >> >> Why are ASHIFT/LSHIFTRT interesting here?  Why are you looking for them? >> >> >> >>> + >>> +/* Find feasibility of extension elimination >>> +   across basic blocks. >>> +   Input: candiate to check the feasibility. >>> +      def_insn of candidate. >>> +   Output: Returns true or false if feasible or not.  */ >> Function comments should read like complete sentences.  Arguments should be in all caps.  There are numerous examples in ree.cc you can follow. >> >> There's no comments in this code which describe the properties of the CFG/blocks you are looking for (domination, post-domination, whatever). It's jsut a series of tests with no clear explanation of what you're looking for or why any particular test exists. >> >> As far as I can tell you're looking at the predecessors of cand->insn and make some decisions based on what you find.  In some cases you return false in others you return true.  But there's zero indication of why you do anything. >> >> You're checking RTL in these cases, but I suspect you really want to be doing some kind of CFG property checks.  But since you haven't described what you're looking for, I can't make suggestions for the right queries to make. >> >> I stopped trying to review the function at this point.  It needs major work.  Let's start with the block/CFG properties you're looking for. We'll then have to go through each section of code and repeat the process. >> >> In fact I might recommend pulling the code which is trying to determine CFG properties into its own function.  Then try to come up with a good name for that function and a good function comment. >> >> THe next chunk of code you start looking at the properties of the candidate insn.  That seems like it ought to break out into its own function as well. >> >> THe process of refactoring, choosing good names and function comments should make the overall intent of this code clearer. >> >> >> > > I am working on this item and will incorporate them. > > Thanks & Regards > Ajit > >> After that's done we'll review that work and perhaps go further. >> >> jeff