From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x134.google.com (mail-lf1-x134.google.com [IPv6:2a00:1450:4864:20::134]) by sourceware.org (Postfix) with ESMTPS id 64E423858D20 for ; Fri, 17 Nov 2023 10:13:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 64E423858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 64E423858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::134 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700216005; cv=none; b=q75Mtte9/iAtK2O4B4KoZd+Q1rGq1+Zn2ZKzDuDg1qaP+NLax/5EUZYh2+L6C6pinIKlwN4EfNqe8h1jirVXIjGGfk5oTlm4DWSOefbwudgsWsnsByZ7FnhX0NagIYGQpBrlEjR4mP1mszXoKbnmt6hzNb/odUQhqfCH93AjzrQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700216005; c=relaxed/simple; bh=6xiHebsF/XGJ6g5KRqO4Zcq4nzqk2PrKt8Q/4i+jHTY=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=W1WcA60Vq4O6nuh4AUdQ8uHQaSie/NfAVjhTOSkjhPi4bwKBP3ZfexudNqOQUynMgAteKRh88rBqG+mVzP8cq+6TnQrtVBQHP/7PYh9plbuoUM753ha7I2/Gd7QbCCr+11pli7+PTQbJAsarqx3YiTknmUABCmweByvkMII5aaQ= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x134.google.com with SMTP id 2adb3069b0e04-507a0907896so2601722e87.2 for ; Fri, 17 Nov 2023 02:13:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700216002; x=1700820802; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=JpWlBgpy04R8JgP7RI/gMyxYYHPNNbvgMY5uDgQGEeA=; b=YpHRR/VlLY9e3TSQ14CZ0+jLidgp1i66BvHUlyyPkvMHCNrVfc7WZSmTDVm1wT9okU VCMz7b1kKtT/qElMfhima7B3AA1oQJpXp/zEruBxSe48zbh32GSxwvB289lvfFt8qiEj pmALVUbjfAI2fJGG2eFG2xnBsxsb9hGZBT63mzIveslWF3d8kzMHMDYEIow3o4JMOQ1Q YmuvvPyyLoEUwC7Z7Jyat6Q5y6ym70HDkBCaeUjJyA2jU5PCXinHKVhOYtO6jkOpe0sG y0pSpdQ4aMNj/gpsKnC10x3KP3UNhha5IJGy3KkHBv5mwk+3MRwAwwQotY+5AOsmQ1bk 4I1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700216002; x=1700820802; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=JpWlBgpy04R8JgP7RI/gMyxYYHPNNbvgMY5uDgQGEeA=; b=TidMqDNkT4bJBUe41U721gcqr8Nx8T8ROUL4+dY4sZ+7CynsduNQh87wSvEnmoTuQT s0HoIaXqcei4oolEqsFw9DdzRGqanY5ekOCFI+2x/XS73pzNrBoNL5BfGHW/osF/5fVe bdDjk7tqO1mV6YN8730xobt+hnncemKzutT1G84h38KZBQdKXSYzlT72tUqbcKo9TKP0 V5Br+yhw0k78TUfKjO/xuPcpt9c0HPWILdZm9nEWes8Nv1o9A+p28GdRq8Huw2jLDJk4 GldvoIg4ZdXnBJl/F6QV+ArOxl14/varxsDHfX6RBqbDERy5TCUIrjJjgdPxSVaAHfIJ 45eA== X-Gm-Message-State: AOJu0YwqIk1hMxzMRnaWlH/lzr1olh88L8YuJsrXv63LLagVjt/dAvSV o9zLINJq53OFfqu9SnfNauavWYedHYHzf0Sxas0= X-Google-Smtp-Source: AGHT+IH0TRHoZKwWFqJ+XFf9bKUQsOJP+NUCpxm5JIz2Op/rlqO+pE6uR3R6avFzevDPHsd7W1SJ2d/EVeMArXD5Tq4= X-Received: by 2002:a05:6512:1316:b0:50a:9fb9:91b with SMTP id x22-20020a056512131600b0050a9fb9091bmr2094303lfu.64.1700216001745; Fri, 17 Nov 2023 02:13:21 -0800 (PST) MIME-Version: 1.0 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> In-Reply-To: <9874e072-747a-39e9-da5e-d88f77b275aa@linux.ibm.com> From: Richard Biener Date: Fri, 17 Nov 2023 11:13:09 +0100 Message-ID: Subject: Re: PING^1 [PATCH v3] sched: Change no_real_insns_p to no_real_nondebug_insns_p [PR108273] To: "Kewen.Lin" 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,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 Fri, Nov 17, 2023 at 10:04=E2=80=AFAM Kewen.Lin wr= ote: > > on 2023/11/15 17:43, Alexander Monakov wrote: > > > > On Wed, 15 Nov 2023, Kewen.Lin wrote: > > > >>>> And I suppose it would be OK to do that. Empty BBs are usually remo= ved by > >>>> CFG cleanup so the situation should only happen in rare corner cases= where > >>>> the fix would be to actually run CFG cleanup ... > >>> > >>> Yeah, sel-sched invokes 'cfg_cleanup (0)' up front, and I suppose tha= t > >>> may be a preferable compromise for sched-rgn as well. > >> > >> Inspired by this discussion, I tested the attached patch 1 which is to= run > >> cleanup_cfg (0) first in haifa_sched_init, it's bootstrapped and > >> regress-tested on x86_64-redhat-linux and powerpc64{,le}-linux-gnu. > > > > 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_in= it > 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. > > > > >> Then I assumed some of the current uses of no_real_insns_p won't encou= nter > >> empty blocks any more, so made a patch 2 with some explicit assertions= , but > >> unfortunately I got ICEs during bootstrapping happens in function > >> compute_priorities. I'm going to investigate it further and post more > >> findings, but just heads-up to ensure if this is on the right track. > > > > I suspect this may be caused by invoking cleanup_cfg too late. > > By looking into some failures, I found that although cleanup_cfg is execu= ted > 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 aff= ect > state transition, for 3, it's the same. For 4, it depends on if we can h= ave > the assumption this kind of empty block doesn't have the chance to have d= ebug > insn (like associated debug insn should be moved along), I'm not sure. F= or 5, > a reduced test case is: > > #include > namespace std { > template <> > basic_istream & > basic_istream::getline (char_type *, streamsize, char_type) __try > { > __streambuf_type *a =3D rdbuf (); > a->sgetc (); > while (traits_type::eq_int_type) > ; > } > __catch (...) {} > } // namespace std > > slim RTL: > > 1: NOTE_INSN_DELETED > 7: NOTE_INSN_BASIC_BLOCK 2 > ... > 15: r137:CCUNS=3Dcmp(r135:DI,r136:DI) > REG_DEAD r136:DI > REG_DEAD r135:DI > 16: pc=3D{(ltu(r137:CCUNS,0))?L24:pc} > REG_DEAD r137:CCUNS > REG_BR_PROB 966367644 > 17: NOTE_INSN_BASIC_BLOCK 3 > 18: r138:DI=3D[r122:DI] > 19: r139:DI=3D[r138:DI+0x48] > REG_DEAD r138:DI > 20: %3:DI=3Dr122:DI > REG_DEAD r122:DI > 21: %12:DI=3Dr139:DI > 22: ctr:DI=3Dr139:DI > REG_DEAD r139:DI > 23: %3:DI=3Dcall [ctr:DI] argc:0 > REG_DEAD ctr:DI > REG_DEAD %12:DI > REG_UNUSED %3:DI > REG_EH_REGION 0x1 > REG_CALL_DECL (nil) > 24: L24: > 25: NOTE_INSN_BASIC_BLOCK 4 > 51: L51: > 48: NOTE_INSN_BASIC_BLOCK 5 > 47: NOTE_INSN_DELETED > 52: pc=3DL51 > 53: barrier > 39: L39: > 41: NOTE_INSN_BASIC_BLOCK 6 > 32: %3:DI=3Dcall [`__cxa_begin_catch'] argc:0 > REG_UNUSED %3:DI > REG_CALL_DECL `__cxa_begin_catch' > REG_EH_REGION 0 > 33: call [`__cxa_end_catch'] argc:0 > REG_CALL_DECL `__cxa_end_catch' > 34: barrier > > ;; basic block 4, loop depth 0, count 10631108 (estimated locally, freq 1= .0000), maybe hot > ;; prev block 3, next block 5, flags: (REACHABLE, RTL) > ;; pred: 3 [always] count:1063111 (estimated locally, freq 0.1000= ) (FALLTHRU) > ;; 2 [90.0% (guessed)] count:9567997 (estimated locally, fr= eq 0.9000) > ;; succ: 5 [always] count:10631108 (estimated locally, freq 1.000= 0) (FALLTHRU) > ;; basic block 5, loop depth 0, count 1073741824 (estimated locally, freq= 101.0000), maybe hot > ;; prev block 4, next block 6, flags: (REACHABLE, RTL, MODIFIED) > ;; pred: 4 [always] count:10631108 (estimated locally, freq 1.000= 0) (FALLTHRU) > ;; 5 [always] count:1073741824 (estimated locally, freq 101= .0000) > ;; succ: 5 [always] count:1073741824 (estimated locally, freq 101= .0000) > > It looks bb 4 can be merged with bb 5, a miss-opt? There can be some oth= er cases > similar to this, either it's miss-opt or not, it seems to affect our assu= mption. > > With the limited findings above so far, I wonder if we still want to go w= ith this > direction that running cleanup_cfg first and make the assumption that the= re is no > such empty block which can cause debug and non-debug state inconsistency? > IMHO it seems risky to make it. Thoughts? Running CFG cleanup shouldn't be the fix to remove such blocks but instead scheduling shouldn't special case empty blocks as they usually do not appea= r. Richard. > > BR, > Kewen