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 282053858023 for ; Thu, 23 Nov 2023 03:02:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 282053858023 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 ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 282053858023 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.156.1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700708572; cv=none; b=Nebryz+0uqgY4p0qvOsYACEXU82uR2ueLoyMRXbPT7fxsFMEr2yVMsLW0mXdWhE0veaZ59ZCi1s/IXrTQMdk9+zvcONZvjuCjNR3vNdxcex1y9zz6+mU6hJGZQfMn4ANpP7PDRvYDVtNJMS8qG/CkW6EsyumcsXoGAzLiqrDkYI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700708572; c=relaxed/simple; bh=UQKVZbRXoe4kYUgBEiuz4EuBKGnlbpZJpG6QnUgDVyU=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=dP2MJFN6ZFEepDLLeBgFA6PGikEDXjS1GlnhTwOGlbrr3ILpdoneTgobqJC2a8LxuyO17TOB0EjZCdTm3wqNS246s0jzmHvQ94AI916Z+v8o+JRer/7Y0stwXCONMa5H0khdAiDEflLizQLvxXnX4LmWavPri7SGzec6VnoA5wo= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353726.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3AN2jwdO017703; Thu, 23 Nov 2023 03:02:46 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=2uN0yBN3iaBdVEhyfJJnyfCxPKcjeAzJ82r5vcMaCeM=; b=KIXlPG2NV4sIO8h5I0JhBX0lfj55tmesMXL4XBttFdYj6hqo+xMDgjIPaPrl7dowqB91 ouW8/B+9W1zkGOrvnultw17iGJwowNmqiz90HL8Bz/aU4TLW77NPC6wNLbzv+8q8oWzJ DpzyeYcjG1GJVWheA/I8Y4QLzaABUcL4RDaq4eFnJ1hI+vODXkYR2dAEUQe942hP32HC NJUrsihMD5nLc/YTOAxUYU5fMNrgKzUpH4ac6TuTpkJkTObwJf7Z+d51B0ImdHK11gNa iqnPkvnnD+L0SDxlN0sQe2pKlwBF+gelNmLykhGpECSYzMPRUkttC5CWegaoxRqpzQaK 3A== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3uhwrc8s7u-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 23 Nov 2023 03:02:45 +0000 Received: from m0353726.ppops.net (m0353726.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 3AN2lKnO020094; Thu, 23 Nov 2023 03:02:45 GMT Received: from ppma13.dal12v.mail.ibm.com (dd.9e.1632.ip4.static.sl-reverse.com [50.22.158.221]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3uhwrc8s6v-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 23 Nov 2023 03:02:45 +0000 Received: from pps.filterd (ppma13.dal12v.mail.ibm.com [127.0.0.1]) by ppma13.dal12v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 3AMNmpOr006196; Thu, 23 Nov 2023 02:36:49 GMT Received: from smtprelay06.fra02v.mail.ibm.com ([9.218.2.230]) by ppma13.dal12v.mail.ibm.com (PPS) with ESMTPS id 3uf9tkm1n9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 23 Nov 2023 02:36:49 +0000 Received: from smtpav02.fra02v.mail.ibm.com (smtpav02.fra02v.mail.ibm.com [10.20.54.101]) by smtprelay06.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 3AN2akCm38535658 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 23 Nov 2023 02:36:46 GMT Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A05C120043; Thu, 23 Nov 2023 02:36:46 +0000 (GMT) Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 906D320040; Thu, 23 Nov 2023 02:36:41 +0000 (GMT) Received: from [9.177.83.221] (unknown [9.177.83.221]) by smtpav02.fra02v.mail.ibm.com (Postfix) with ESMTP; Thu, 23 Nov 2023 02:36:41 +0000 (GMT) Message-ID: <814d768f-b858-f377-0155-fdefed0aa078@linux.ibm.com> Date: Thu, 23 Nov 2023 10:36:39 +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: PING^1 [PATCH v3] sched: Change no_real_insns_p to no_real_nondebug_insns_p [PR108273] Content-Language: en-US To: Richard Biener Cc: Alexander Monakov , Jeff Law , Maxim Kuvyrkov , GCC Patches , Richard Sandiford , Jeff Law , Vladimir Makarov , zhroma@ispras.ru, Andrey Belevantsev , Segher Boessenkool , Peter Bergner , Michael Meissner , Alexandre Oliva References: <85b4098e-a72f-d013-ff17-8097971f71ba@linux.ibm.com> <09FEFDAE-698B-4B06-A896-8088B9B31539@linaro.org> <4675c26c-f230-b6d6-27c5-bc9f74736e38@linux.ibm.com> <41a4d065-c4b6-4a67-adf0-e84e942616c7@gmail.com> <93ce3468-a1ee-e77c-cbeb-a8c67a303bf9@ispras.ru> <7eb725d9-de7c-87ba-5ebd-f2e1485c5854@ispras.ru> <475af219-f250-a0f4-78b0-998f96fb24aa@linux.ibm.com> <9874e072-747a-39e9-da5e-d88f77b275aa@linux.ibm.com> <434b7d49-808e-5254-b023-a7e1dad29f81@ispras.ru> <1f407c4c-fb0a-c9c8-6438-144cfa77fd4b@linux.ibm.com> From: "Kewen.Lin" In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: pKByW4JbXWBKks4azb88PMWJaPmwk34K X-Proofpoint-ORIG-GUID: _lMsyeTMI9NU6TBNjtU4W-zsUvmvysyh X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.987,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-11-22_18,2023-11-22_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 malwarescore=0 spamscore=0 suspectscore=0 phishscore=0 impostorscore=0 priorityscore=1501 lowpriorityscore=0 mlxlogscore=999 mlxscore=0 bulkscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311060000 definitions=main-2311230021 X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,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: on 2023/11/22 18:25, Richard Biener wrote: > On Wed, Nov 22, 2023 at 10:31 AM Kewen.Lin wrote: >> >> on 2023/11/17 20:55, Alexander Monakov wrote: >>> >>> On Fri, 17 Nov 2023, Kewen.Lin wrote: >>>>> I don't think you can run cleanup_cfg after sched_init. I would suggest >>>>> to put it early in schedule_insns. >>>> >>>> Thanks for the suggestion, I placed it at the beginning of haifa_sched_init >>>> instead, since schedule_insns invokes haifa_sched_init, although the >>>> calls rgn_setup_common_sched_info and rgn_setup_sched_infos are executed >>>> ahead but they are all "setup" functions, shouldn't affect or be affected >>>> by this placement. >>> >>> I was worried because sched_init invokes df_analyze, and I'm not sure if >>> cfg_cleanup can invalidate it. >> >> Thanks for further explaining! By scanning cleanup_cfg, it seems that it >> considers df, like compact_blocks checks df, try_optimize_cfg invokes >> df_analyze etc., but I agree that moving cleanup_cfg before sched_init >> makes more sense. >> >>> >>>>> I suspect this may be caused by invoking cleanup_cfg too late. >>>> >>>> By looking into some failures, I found that although cleanup_cfg is executed >>>> there would be still some empty blocks left, by analyzing a few failures there >>>> are at least such cases: >>>> 1. empty function body >>>> 2. block holding a label for return. >>>> 3. block without any successor. >>>> 4. block which becomes empty after scheduling some other block. >>>> 5. block which looks mergeable with its always successor but left. >>>> ... >>>> >>>> For 1,2, there is one single successor EXIT block, I think they don't affect >>>> state transition, for 3, it's the same. For 4, it depends on if we can have >>>> the assumption this kind of empty block doesn't have the chance to have debug >>>> insn (like associated debug insn should be moved along), I'm not sure. For 5, >>>> a reduced test case is: >>> >>> Oh, I should have thought of cases like these, really sorry about the slip >>> of attention, and thanks for showing a testcase for item 5. As Richard as >>> saying in his response, cfg_cleanup cannot be a fix here. The thing to check >>> would be changing no_real_insns_p to always return false, and see if the >>> situation looks recoverable (if it breaks bootstrap, regtest statistics of >>> a non-bootstrapped compiler are still informative). >> >> As you suggested, I forced no_real_insns_p to return false all the time, some >> issues got exposed, almost all of them are asserting NOTE_P insn shouldn't be >> encountered in those places, so the adjustments for most of them are just to >> consider NOTE_P or this kind of special block and so on. One draft patch is >> attached, it can be bootstrapped and regress-tested on ppc64{,le} and x86. >> btw, it's without the previous cfg_cleanup adjustment (hope it can get more >> empty blocks and expose more issues). The draft isn't qualified for code >> review but I hope it can provide some information on what kinds of changes >> are needed for the proposal. If this is the direction which we all agree on, >> I'll further refine it and post a formal patch. One thing I want to note is >> that this patch disable one assertion below: >> >> diff --git a/gcc/sched-rgn.cc b/gcc/sched-rgn.cc >> index e5964f54ead..abd334864fb 100644 >> --- a/gcc/sched-rgn.cc >> +++ b/gcc/sched-rgn.cc >> @@ -3219,7 +3219,7 @@ schedule_region (int rgn) >> } >> >> /* Sanity check: verify that all region insns were scheduled. */ >> - gcc_assert (sched_rgn_n_insns == rgn_n_insns); >> + // gcc_assert (sched_rgn_n_insns == rgn_n_insns); >> >> sched_finish_ready_list (); >> >> Some cases can cause this assertion to fail, it's due to the mismatch on >> to-be-scheduled and scheduled insn counts. The reason why it happens is that >> one block previously has only one INSN_P but while scheduling some other blocks >> it gets moved as well then we ends up with an empty block so that the only >> NOTE_P insn was counted then, but since this block isn't empty initially and >> NOTE_P gets skipped in a normal block, the count to-be-scheduled can't count >> it in. It can be fixed with special-casing this kind of block for counting >> like initially recording which block is empty and if a block isn't recorded >> before then fix up the count for it accordingly. I'm not sure if someone may >> have an argument that all the complication make this proposal beaten by >> previous special-casing debug insn approach, looking forward to more comments. > > Just a comment that the NOTE_P thing is odd - do we only ever have those for > otherwise empty BBs? How are they skipped otherwise (and why does that not > work for otherwise empty BBs)? Yes, previously (bypassing empty BBs) there is no chance to encounter NOTE_P when scheduling insns, as for notes in normal BBs, when setting up the head and tail, some are skipped (like get_ebb_head_tail), and there are also some special handlings remove_notes and unlink_bb_notes to guarantee they are gone. By disabling empty BB bypassing, all empty BBs will be actually uniformed as (head == tail && NOTE_P (head)), we have to deal with NOTE_P. BR, Kewen