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 A79C83AA7C8E for ; Thu, 24 Jun 2021 09:26:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A79C83AA7C8E Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 15O93NaS115320; Thu, 24 Jun 2021 05:26:45 -0400 Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 39chyaabw9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 24 Jun 2021 05:26:45 -0400 Received: from m0098409.ppops.net (m0098409.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 15O94JG0118500; Thu, 24 Jun 2021 05:26:44 -0400 Received: from ppma03ams.nl.ibm.com (62.31.33a9.ip4.static.sl-reverse.com [169.51.49.98]) by mx0a-001b2d01.pphosted.com with ESMTP id 39chyaabv3-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 24 Jun 2021 05:26:44 -0400 Received: from pps.filterd (ppma03ams.nl.ibm.com [127.0.0.1]) by ppma03ams.nl.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 15O9DM9V018126; Thu, 24 Jun 2021 09:26:42 GMT Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by ppma03ams.nl.ibm.com with ESMTP id 399878aetu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 24 Jun 2021 09:26:42 +0000 Received: from b06wcsmtp001.portsmouth.uk.ibm.com (b06wcsmtp001.portsmouth.uk.ibm.com [9.149.105.160]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 15O9QesP23527866 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 24 Jun 2021 09:26:40 GMT Received: from b06wcsmtp001.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id EA43AA4060; Thu, 24 Jun 2021 09:26:39 +0000 (GMT) Received: from b06wcsmtp001.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 20BF6A4054; Thu, 24 Jun 2021 09:26:38 +0000 (GMT) Received: from KewenLins-MacBook-Pro.local (unknown [9.197.248.48]) by b06wcsmtp001.portsmouth.uk.ibm.com (Postfix) with ESMTP; Thu, 24 Jun 2021 09:26:37 +0000 (GMT) Subject: Re: predcom: Refactor more by encapsulating global states To: Martin Sebor Cc: GCC Patches , Segher Boessenkool , Bill Schmidt , Richard Biener References: <6f695498-7f72-fe2f-b88b-4240f0d4569a@linux.ibm.com> <2b1c6017-08b4-e96e-cc0d-b229e912ea0e@linux.ibm.com> From: "Kewen.Lin" Message-ID: Date: Thu, 24 Jun 2021 17:26:36 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US X-TM-AS-GCONF: 00 X-Proofpoint-GUID: LvBk1TCG5dAHBcXBr1YsilofjrjApKxE X-Proofpoint-ORIG-GUID: bzaNzUXWGM7MBlqFCMB_hL4xUr3_zKdE Content-Transfer-Encoding: 8bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391, 18.0.790 definitions=2021-06-24_06:2021-06-24, 2021-06-24 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 mlxscore=0 spamscore=0 lowpriorityscore=0 suspectscore=0 bulkscore=0 adultscore=0 clxscore=1015 malwarescore=0 impostorscore=0 phishscore=0 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104190000 definitions=main-2106240048 X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, KAM_SHORT, NICE_REPLY_A, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Jun 2021 09:26:50 -0000 Hi Martin, on 2021/6/23 上午12:14, Martin Sebor wrote: > On 6/21/21 8:35 PM, Kewen.Lin wrote: >> Hi Richi and Martin, >> >>>> >>>> Thanks Richi!  One draft (not ready for review) is attached for the further >>>> discussion.  It follows the idea of RAII-style cleanup.  I noticed that >>>> Martin suggested stepping forward to make tree_predictive_commoning_loop >>>> and its callees into one class (Thanks Martin), since there are not many >>>> this kind of C++-style work functions, I want to double confirm which option >>>> do you guys prefer? >>>> >>> >>> Such general cleanup is of course desired - Giuliano started some of it within >>> GSoC two years ago in the attempt to thread the compilation process.  The >>> cleanup then helps to get rid of global state which of course interferes here >>> (and avoids unnecessary use of TLS vars). >>> >>> So yes, encapsulating global state into a class and making accessors >>> member functions is something that is desired (but a lot of mechanical >>> work). >>> >>> Thanks >>> Richard. >>> >>> I meant that not necessarily as something to include in this patch >>> but as a suggestion for a future improvement.  If you'd like to >>> tackle it at any point that would be great of course   In any >>> event, thanks for double-checking! >>> >>> The attached patch looks good to me as well (more for the sake of >>> style than anything else, declaring the class copy ctor and copy assignment = delete would > make it clear it's not meant to be >>> copied, although in this case it's unlikely to make a practical >>> difference). >>> >>> Martin. >> >> >> Thanks for your explanation!  Sorry for the late response. >> As the way to encapsulate global state into a class and making accessors >> member functions looks more complete, I gave up the RAII draft and >> switched onto this way. >> >> This patch is to encapsulate global states into a class and >> making their accessors as member functions, remove some >> consequent useless clean up code, and do some clean up with >> RAII. > > Nice! > > A further improvement worth considering (if you're so inclined :) > is replacing the pcom_worker vec members with auto_vec (obviating > having to explicitly release them) and for the same reason also > replacing the comp_ptrs bare pointer members with auto_vecs. > There may be other opportunities to do the same in individual > functions (I'd look to get rid of as many calls to functions > like XNEW()/XNEWVEC() and free() use auto_vec instead). > > An unrelated but worthwhile change is to replace the FOR_EACH_ > loops with C++ 11 range loops, analogously to: > https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572315.html > > Finally, the only loosely followed naming convention for member > variables is to start them with the m_ prefix. > > These just suggestions that could be done in a followup, not > something I would consider prerequisite for accepting the patch > as is if I were in a position to make such a decision. > Many thanks for all the great suggestions! I'll deal with them in a follow up patch. BR, Kewen