diff mbox

Fix checking on MAX_PENDING_LIST_LENGTH

Message ID 000001cffcc7$00a72840$01f578c0$@arm.com
State New
Headers show

Commit Message

Bin Cheng Nov. 10, 2014, 9:16 a.m. UTC 
Hi,
There is parameter max-pending-list-length in gcc scheduler, but the
parameter is checked using greater than condition.  As a result, the real
max pending list length is actually "max-pending-list-length + 1".  This
patch fixes this by using ">=" rather than ">" comparison operator.  Though
it is kind of nit-picking, I want to change this: a) it breaks sched-fusion
because the 33rd couldn't be paired; b) when sched-fusion tries to sort many
consecutive stores, it breaks dcache line alignment at large probability.  I
mean without cache sensitive optimizer, GCC breaks dcache line alignment
randomly, but 33 is definitely worse than 32.  Of course, this only happens
in very restricted case.

Bootstrap and test on x86_64.  Is it OK?

2014-11-10  Bin Cheng  <bin.cheng@arm.com>

	* sched-deps.c (sched_analyze_1): Check pending list if it is not
	less than MAX_PENDING_LIST_LENGTH.
	(sched_analyze_2, sched_analyze_insn, deps_analyze_insn): Ditto.

Comments

Richard Biener Nov. 10, 2014, 12:07 p.m. UTC | #1 
On Mon, Nov 10, 2014 at 10:16 AM, Bin Cheng <bin.cheng@arm.com> wrote:
> Hi,
> There is parameter max-pending-list-length in gcc scheduler, but the
> parameter is checked using greater than condition.  As a result, the real
> max pending list length is actually "max-pending-list-length + 1".  This
> patch fixes this by using ">=" rather than ">" comparison operator.  Though
> it is kind of nit-picking, I want to change this: a) it breaks sched-fusion
> because the 33rd couldn't be paired; b) when sched-fusion tries to sort many
> consecutive stores, it breaks dcache line alignment at large probability.  I
> mean without cache sensitive optimizer, GCC breaks dcache line alignment
> randomly, but 33 is definitely worse than 32.  Of course, this only happens
> in very restricted case.
>
> Bootstrap and test on x86_64.  Is it OK?

Ok.

Thanks,
Richard.

> 2014-11-10  Bin Cheng  <bin.cheng@arm.com>
>
>         * sched-deps.c (sched_analyze_1): Check pending list if it is not
>         less than MAX_PENDING_LIST_LENGTH.
>         (sched_analyze_2, sched_analyze_insn, deps_analyze_insn): Ditto.
diff mbox

Patch

Index: gcc/sched-deps.c
===================================================================
--- gcc/sched-deps.c	(revision 217273)
+++ gcc/sched-deps.c	(working copy)
@@ -2504,7 +2504,7 @@  sched_analyze_1 (struct deps_desc *deps, rtx x, rt
       /* Pending lists can't get larger with a readonly context.  */
       if (!deps->readonly
           && ((deps->pending_read_list_length + deps->pending_write_list_length)
-              > MAX_PENDING_LIST_LENGTH))
+              >= MAX_PENDING_LIST_LENGTH))
 	{
 	  /* Flush all pending reads and writes to prevent the pending lists
 	     from getting any larger.  Insn scheduling runs too slowly when
@@ -2722,7 +2722,7 @@  sched_analyze_2 (struct deps_desc *deps, rtx x, rt
 	  {
 	    if ((deps->pending_read_list_length
 		 + deps->pending_write_list_length)
-		> MAX_PENDING_LIST_LENGTH
+		>= MAX_PENDING_LIST_LENGTH
 		&& !DEBUG_INSN_P (insn))
 	      flush_pending_lists (deps, insn, true, true);
 	    add_insn_mem_dependence (deps, true, insn, x);
@@ -3227,8 +3227,8 @@  sched_analyze_insn (struct deps_desc *deps, rtx x,
 	  EXECUTE_IF_SET_IN_REG_SET (reg_pending_clobbers, 0, i, rsi)
 	    {
 	      struct deps_reg *reg_last = &deps->reg_last[i];
-	      if (reg_last->uses_length > MAX_PENDING_LIST_LENGTH
-		  || reg_last->clobbers_length > MAX_PENDING_LIST_LENGTH)
+	      if (reg_last->uses_length >= MAX_PENDING_LIST_LENGTH
+		  || reg_last->clobbers_length >= MAX_PENDING_LIST_LENGTH)
 		{
 		  add_dependence_list_and_free (deps, insn, &reg_last->sets, 0,
 						REG_DEP_OUTPUT, false);
@@ -3661,7 +3661,7 @@  deps_analyze_insn (struct deps_desc *deps, rtx_ins
                && sel_insn_is_speculation_check (insn)))
         {
           /* Keep the list a reasonable size.  */
-          if (deps->pending_flush_length++ > MAX_PENDING_LIST_LENGTH)
+          if (deps->pending_flush_length++ >= MAX_PENDING_LIST_LENGTH)
             flush_pending_lists (deps, insn, true, true);
           else
 	    deps->pending_jump_insns