public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Kewen.Lin" <linkw@linux.ibm.com>
To: Alexander Monakov <amonakov@ispras.ru>,
	Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
	Richard Guenther <richard.guenther@gmail.com>,
	Richard Sandiford <richard.sandiford@arm.com>,
	Jeff Law <jlaw@ventanamicro.com>,
	Vladimir Makarov <vmakarov@redhat.com>,
	zhroma@ispras.ru, Andrey Belevantsev <abel@ispras.ru>,
	Segher Boessenkool <segher@kernel.crashing.org>,
	Peter Bergner <bergner@linux.ibm.com>,
	Michael Meissner <meissner@linux.ibm.com>
Subject: [PATCH] sched: Remove debug counter sched_block
Date: Wed, 15 Nov 2023 17:01:46 +0800	[thread overview]
Message-ID: <be4a62d2-32eb-eb3b-56de-801d602e364d@linux.ibm.com> (raw)
In-Reply-To: <b0cf2867-ef64-9d61-bae0-b573689fe62d@ispras.ru>

Hi,

on 2023/11/10 01:40, Alexander Monakov wrote:

> I agree with the concern. I hoped that solving the problem by skipping the BB
> like the (bit-rotted) debug code needs to would be a minor surgery. As things
> look now, it may be better to remove the non-working sched_block debug counter
> entirely and implement a good solution for the problem at hand.
> 

According to this comment, I made and tested the below patch to remove the
problematic debug counter:

Subject: [PATCH] sched: Remove debug counter sched_block

Currently the debug counter sched_block doesn't work well
since we create dependencies for some insns and those
dependencies are expected to be resolved during scheduling
insns but they can get skipped once we are skipping some
block while respecting sched_block debug counter.

For example, for the below test case:
--
int a, b, c, e, f;
float d;

void
g ()
{
  float h, i[1];
  for (; f;)
    if (c)
      {
	d *e;
	if (b)
	  {
	    float *j = i;
	    j[0] = 0;
	  }
	h = d;
      }
  if (h)
    a = i[0];
}
--
ICE occurs with option "-O2 -fdbg-cnt=sched_block:1".

As the discussion in [1], it seems that we think this debug
counter is useless and can be removed.  It's also implied
that if it's useful and used often, the above issue should
have been cared about and resolved earlier.  So this patch
is to remove this debug counter.

Bootstrapped and regtested on x86_64-redhat-linux and
powerpc64{,le}-linux-gnu.

[1] https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635852.html

Is it ok for trunk?

BR,
Kewen
-----

gcc/ChangeLog:

	* dbgcnt.def (sched_block): Remove.
	* sched-rgn.cc (schedule_region): Remove the support of debug count
	sched_block.
---
 gcc/dbgcnt.def   |  1 -
 gcc/sched-rgn.cc | 19 ++++++-------------
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/gcc/dbgcnt.def b/gcc/dbgcnt.def
index 871cbf75d93..a8c4e61e13d 100644
--- a/gcc/dbgcnt.def
+++ b/gcc/dbgcnt.def
@@ -198,7 +198,6 @@ DEBUG_COUNTER (pre_insn)
 DEBUG_COUNTER (prefetch)
 DEBUG_COUNTER (registered_jump_thread)
 DEBUG_COUNTER (sched2_func)
-DEBUG_COUNTER (sched_block)
 DEBUG_COUNTER (sched_breakdep)
 DEBUG_COUNTER (sched_func)
 DEBUG_COUNTER (sched_insn)
diff --git a/gcc/sched-rgn.cc b/gcc/sched-rgn.cc
index e5964f54ead..1c8acf5068a 100644
--- a/gcc/sched-rgn.cc
+++ b/gcc/sched-rgn.cc
@@ -3198,20 +3198,13 @@ schedule_region (int rgn)
       current_sched_info->queue_must_finish_empty = current_nr_blocks == 1;

       curr_bb = first_bb;
-      if (dbg_cnt (sched_block))
-        {
-	  int saved_last_basic_block = last_basic_block_for_fn (cfun);
+      int saved_last_basic_block = last_basic_block_for_fn (cfun);

-	  schedule_block (&curr_bb, bb_state[first_bb->index]);
-	  gcc_assert (EBB_FIRST_BB (bb) == first_bb);
-	  sched_rgn_n_insns += sched_n_insns;
-	  realloc_bb_state_array (saved_last_basic_block);
-	  save_state_for_fallthru_edge (last_bb, curr_state);
-        }
-      else
-        {
-          sched_rgn_n_insns += rgn_n_insns;
-        }
+      schedule_block (&curr_bb, bb_state[first_bb->index]);
+      gcc_assert (EBB_FIRST_BB (bb) == first_bb);
+      sched_rgn_n_insns += sched_n_insns;
+      realloc_bb_state_array (saved_last_basic_block);
+      save_state_for_fallthru_edge (last_bb, curr_state);

       /* Clean up.  */
       if (current_nr_blocks > 1)
--
2.39.1

  parent reply	other threads:[~2023-11-15  9:02 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-25  2:45 [PATCH v3] sched: Change no_real_insns_p to no_real_nondebug_insns_p [PR108273] Kewen.Lin
2023-11-08  2:49 ` PING^1 " Kewen.Lin
2023-11-08  9:45   ` Richard Sandiford
2023-11-09  9:10   ` Maxim Kuvyrkov
2023-11-09 17:40     ` Alexander Monakov
2023-11-10  1:57       ` Kewen.Lin
2023-11-10  3:49         ` Jeff Law
2023-11-10 11:25           ` Alexander Monakov
2023-11-10 13:32             ` Richard Biener
2023-11-10 14:18               ` Alexander Monakov
2023-11-10 14:20                 ` Richard Biener
2023-11-10 14:41                   ` Alexander Monakov
2023-11-15  9:12                     ` Kewen.Lin
2023-11-15  9:43                       ` Alexander Monakov
2023-11-17  9:03                         ` Kewen.Lin
2023-11-17 10:13                           ` Richard Biener
2023-11-17 12:55                           ` Alexander Monakov
2023-11-22  9:30                             ` Kewen.Lin
2023-11-22 10:25                               ` Richard Biener
2023-11-23  2:36                                 ` Kewen.Lin
2023-11-23  8:20                                   ` Richard Biener
2023-11-23  9:09                                     ` Kewen.Lin
2023-12-12  7:02                               ` [PATCH draft v2] sched: Don't skip empty block in scheduling [PR108273] Kewen.Lin
2023-11-10  3:49       ` PING^1 [PATCH v3] sched: Change no_real_insns_p to no_real_nondebug_insns_p [PR108273] Jeff Law
2023-11-15  9:01       ` Kewen.Lin [this message]
2023-12-12  6:17         ` PING^1 [PATCH] sched: Remove debug counter sched_block Kewen.Lin
2023-12-20 20:43           ` Jeff Law
2023-12-21  5:46             ` Kewen.Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=be4a62d2-32eb-eb3b-56de-801d602e364d@linux.ibm.com \
    --to=linkw@linux.ibm.com \
    --cc=abel@ispras.ru \
    --cc=amonakov@ispras.ru \
    --cc=bergner@linux.ibm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jlaw@ventanamicro.com \
    --cc=maxim.kuvyrkov@linaro.org \
    --cc=meissner@linux.ibm.com \
    --cc=richard.guenther@gmail.com \
    --cc=richard.sandiford@arm.com \
    --cc=segher@kernel.crashing.org \
    --cc=vmakarov@redhat.com \
    --cc=zhroma@ispras.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).