This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

PR80357: Negative register pressure


In the PR testcase, there were two instructions that had
a large number of insn_reg_use records for the same register.
model_recompute was instead expecting the records to be unique
and so decremented the register pressure for each one.  We then
ended up with a negative pressure.

I think the records *should* be unique here, and that this is
really a bug in the generic -fsched-pressure code.  Making them
unique could be too invasive for GCC 7 though.  There are at
least two problems:

(1) sched-deps.c uses rtx_insn_lists instead of bitmaps to record
    the set of instructions that use a live register.
    sched-rgn.c then propagates this information between blocks
    in a region using list concatenation:

      succ_rl->uses = concat_INSN_LIST (pred_rl->uses, succ_rl->uses);

    So dependencies for common predecessors will appear multiple
    times in the list.

    In this case (and for the PR), it might be enough to make
    setup_insn_reg_uses detect duplicate entries.  However...

(2) setup_insn_reg_uses adds entries for all queued uses of a register
    at the point that the register is expected to die.  It looks like
    this doesn't work reliably for REG_INC registers: if a register R
    is auto-incremented in I1 and then used for the last time in I2,
    setup_insn_reg_uses will first treat the original R as dying
    in I1 (rightly IMO).  But that use of R in I1 is still in the
    reg_last->uses list when processing I2, so setup_insn_reg_uses
    adds it a second time.

There might be more reasons for multiple records: I stopped looking
at this point.

I think for GCC 7 it'd be more pragmatic to live with the duplicate
entries and keep the fix specific to SCHED_PRESSURE_MODEL.

Tested on aarch64-linux-gnu (which uses SCHED_PRESSURE_MODEL by default)
and on x86_64-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
	PR rtl-optimization/80357
	* haifa-sched.c (tmp_bitmap): New variable.
	(model_recompute): Handle duplicate use records.
	(alloc_global_sched_pressure_data): Initialize tmp_bitmap.
	(free_global_sched_pressure_data): Free it.

gcc/testsuite/
	PR rtl-optimization/80357
	* gcc.c-torture/compile/pr80357.c: New test.

diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
index 07de1bb..0ebf110 100644
--- a/gcc/haifa-sched.c
+++ b/gcc/haifa-sched.c
@@ -930,6 +930,9 @@ static bitmap saved_reg_live;
 /* Registers mentioned in the current region.  */
 static bitmap region_ref_regs;
 
+/* Temporary bitmap used for SCHED_PRESSURE_MODEL.  */
+static bitmap tmp_bitmap;
+
 /* Effective number of available registers of a given class (see comment
    in sched_pressure_start_bb).  */
 static int sched_class_regs_num[N_REG_CLASSES];
@@ -2135,10 +2138,11 @@ model_recompute (rtx_insn *insn)
      registers that will be born in the range [model_curr_point, POINT).  */
   num_uses = 0;
   num_pending_births = 0;
+  bitmap_clear (tmp_bitmap);
   for (use = INSN_REG_USE_LIST (insn); use != NULL; use = use->next_insn_use)
     {
       new_last = model_last_use_except (use);
-      if (new_last < point)
+      if (new_last < point && bitmap_set_bit (tmp_bitmap, use->regno))
 	{
 	  gcc_assert (num_uses < ARRAY_SIZE (uses));
 	  uses[num_uses].last_use = new_last;
@@ -7241,6 +7245,8 @@ alloc_global_sched_pressure_data (void)
 	  saved_reg_live = BITMAP_ALLOC (NULL);
 	  region_ref_regs = BITMAP_ALLOC (NULL);
 	}
+      if (sched_pressure == SCHED_PRESSURE_MODEL)
+	tmp_bitmap = BITMAP_ALLOC (NULL);
 
       /* Calculate number of CALL_SAVED_REGS and FIXED_REGS in register classes
 	 that we calculate register pressure for.  */
@@ -7274,6 +7280,8 @@ free_global_sched_pressure_data (void)
 	  BITMAP_FREE (region_ref_regs);
 	  BITMAP_FREE (saved_reg_live);
 	}
+      if (sched_pressure == SCHED_PRESSURE_MODEL)
+	BITMAP_FREE (tmp_bitmap);
       BITMAP_FREE (curr_reg_live);
       free (sched_regno_pressure_class);
     }
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr80357.c b/gcc/testsuite/gcc.c-torture/compile/pr80357.c
new file mode 100644
index 0000000..a3c6ab0
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr80357.c
@@ -0,0 +1,18 @@
+typedef char a;
+a b, c;
+int d, e;
+void f(void *g) { *(volatile int *)g; }
+void j() {
+  a h, i;
+  for (; b; b += 2) {
+    d = b;
+    i = i >> b;
+    if (i)
+      continue;
+    f(&c + (b >> 2));
+    h = 0;
+    for (; h < 8 / 2; h++)
+      if (i << h)
+        e = 0;
+  }
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]