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 2CE803858C3A for ; Tue, 21 Sep 2021 06:14:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2CE803858C3A Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 18L4dOEa001391; Tue, 21 Sep 2021 02:13:59 -0400 Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 3b7680uyb0-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 21 Sep 2021 02:13:58 -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 18L5nkHZ014659; Tue, 21 Sep 2021 02:13:58 -0400 Received: from ppma05fra.de.ibm.com (6c.4a.5195.ip4.static.sl-reverse.com [149.81.74.108]) by mx0a-001b2d01.pphosted.com with ESMTP id 3b7680uyac-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 21 Sep 2021 02:13:57 -0400 Received: from pps.filterd (ppma05fra.de.ibm.com [127.0.0.1]) by ppma05fra.de.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 18L6DAZo025509; Tue, 21 Sep 2021 06:13:55 GMT Received: from b06avi18878370.portsmouth.uk.ibm.com (b06avi18878370.portsmouth.uk.ibm.com [9.149.26.194]) by ppma05fra.de.ibm.com with ESMTP id 3b57r8y50e-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 21 Sep 2021 06:13:55 +0000 Received: from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com [9.149.105.61]) by b06avi18878370.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 18L6962w49742248 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 21 Sep 2021 06:09:06 GMT Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0D1CC11C058; Tue, 21 Sep 2021 06:13:53 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3E71711C050; Tue, 21 Sep 2021 06:13:50 +0000 (GMT) Received: from KewenLins-MacBook-Pro.local (unknown [9.200.63.17]) by d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP; Tue, 21 Sep 2021 06:13:49 +0000 (GMT) Subject: Re: [PATCH v3] ipa-inline: Add target info into fn summary [PR102059] To: Martin Jambor Cc: GCC Patches , Richard Biener , Jan Hubicka , =?UTF-8?Q?Martin_Li=c5=a1ka?= , Florian Weimer , Bill Schmidt , Segher Boessenkool References: <20210917141419.GR1583@gate.crashing.org> From: "Kewen.Lin" Message-ID: <3e671671-3b76-8e2e-71c6-97d26b1dbef7@linux.ibm.com> Date: Tue, 21 Sep 2021 14:13:48 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=gbk Content-Language: en-US Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: 6U5qc_J6egciN5_4YzDks3qqkyqFE4_h X-Proofpoint-GUID: 2Yy2PpAuOHXvPnp3Y2obEEl913Si8YH6 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.182.1,Aquarius:18.0.790,Hydra:6.0.391,FMLib:17.0.607.475 definitions=2021-09-21_01,2021-09-20_01,2020-04-07_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 mlxlogscore=999 malwarescore=0 priorityscore=1501 impostorscore=0 lowpriorityscore=0 clxscore=1015 bulkscore=0 adultscore=0 mlxscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109030001 definitions=main-2109210039 X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, MIME_CHARSET_FARAWAY, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Tue, 21 Sep 2021 06:14:01 -0000 Hi Martin, Thanks for the review. on 2021/9/18 ÏÂÎç7:31, Martin Jambor wrote: > Hi, > > On Fri, Sep 17 2021, Segher Boessenkool wrote: >> On Fri, Sep 17, 2021 at 05:42:38PM +0800, Kewen.Lin wrote: >>> Against v2 [2], this v3 addressed Martin's review comments: >>> - Replace HWI auto_vec with unsigned int for target_info >>> to avoid overkill (also Segher's comments), adjust some >>> places need to be updated for this change. >> >> I'd have used a single HWI (always 64 bits), but an int (always at least >> 32 bits in GCC) is fine as well, sure. > > Let's have it as unsigned int then (in a separate thread I was > suggesting to go even smaller). > >> That easily fits one line. (Many more examples here btw). >> >>> +bool >>> +rs6000_fn_has_any_of_these_mask_bits (enum rs6000_builtins code, >>> + HOST_WIDE_INT mask) >>> +{ >>> + gcc_assert (code < RS6000_BUILTIN_COUNT); >> >> We don't have this assert anywhere else, so lose it here as well? >> >> If we want such checking we should make an inline accessor function for >> this, and check it there. But we already do a check in >> rs6000_builtin_decl (and one in def_builtin, but that one has an >> off-by-one error in it). >> >>> +extern bool rs6000_fn_has_any_of_these_mask_bits (enum rs6000_builtins code, >>> + HOST_WIDE_INT mask); >> >> The huge unwieldy name suggests it might not be the best abstraction you >> could use, btw ;-) >> >>> +static bool >>> +rs6000_update_ipa_fn_target_info (unsigned int &info, const gimple *stmt) >>> +{ >>> + /* Assume inline asm can use any instruction features. */ >>> + if (gimple_code (stmt) == GIMPLE_ASM) >> >> This should be fine for HTM, but it may be a bit *too* pessimistic for >> other features. We'll see when we get there :-) >> >>> +@deftypefn {Target Hook} bool TARGET_NEED_IPA_FN_TARGET_INFO (const_tree @var{decl}, unsigned int& @var{info}) >>> +Allow target to check early whether it is necessary to analyze all gimple >>> +statements in the given function to update target specific information for >>> +inlining. See hook @code{update_ipa_fn_target_info} for usage example of >> [ ... ] >>> +The default version of this hook returns false. >> >> And that is really the only reason to have this premature optimisation: >> targets that do not care do not have to pay the price, however trivial >> that price may be, which is a good idea politically ;-) >> >>> +/* { dg-final { scan-tree-dump-times "Inlining foo/\[0-9\]* " 1 "einline"} } */ >> >> If you use {} instead of "" you don't need the backslashes. >> >>> +default_update_ipa_fn_target_info (uint16_t &, const gimple *) >> >> I'm surprised the compiler didn't warn about this btw. >> >> The rs6000 parts are okay for trunk (with the trivial cleanups please). >> Thanks! > >> +/* By default, return false to not need to collect any target information >> + for inlining. Target maintainer should re-define the hook if the >> + target want to take advantage of it. */ >> + >> +bool >> +default_need_ipa_fn_target_info (const_tree, uint16_t &) >> +{ >> + return false; >> +} >> + >> +bool >> +default_update_ipa_fn_target_info (uint16_t &, const gimple *) >> +{ >> + return false; >> +} > > The parameters have uint16_t type here but you apparently decided to use unsigned int everywhere else, you probably forgot to change them here too. Yeah, I did forget to change them. :( Thanks for catching! > the IPA bits are OK too (after the type mismatch is fixed). > Thanks! BR, Kewen