diff mbox

[lra,committed] Fix PR 69447

Message ID 56A94228.3070409@redhat.com
State New
Headers show

Commit Message

Richard Henderson Jan. 27, 2016, 10:18 p.m. UTC
This PR appears to be related to PR 66424, which Vlad fixed back in September,
but different in that the insn being rematerialized used the entire DImode
pseudo instead of an SImode subreg of a DImode pseudo.

The effect was the same, however, in that it lengthened the lifetime of one
half of the double-word pseudo, causing it to conflict with the coloring set up
by IRA.

While this solves the problem by refusing to remat any double-word pseudo that
has ever had one of its subreg taken, a more complete solution is to also sync
IRA and LRA so that both of them track lifetimes of subregs.  Not for now,
obviously.

Tested on x86_64, i686, and armv7hf-linux.
Approved by Vlad in the PR.


r~
PR rtl-opt/69447
	* lra-remat.c (subreg_regs): New.
	(dump_candidates_and_remat_bb_data): Dump it.
	(operand_to_remat): Reject if operand in subreg_regs.
	(set_bb_regs): Collect subreg_regs.
	(lra_remat): Init and free subreg_regs.  Compute
	calculate_local_reg_remat_bb_data before create_cands.

testsuite/
	* gcc.c-torture/execute/pr69447.c: New test.
diff mbox

Patch

diff --git a/gcc/lra-remat.c b/gcc/lra-remat.c
index 6f490b9..4d8099f 100644
--- a/gcc/lra-remat.c
+++ b/gcc/lra-remat.c
@@ -77,6 +77,9 @@  static int call_used_regs_arr[FIRST_PSEUDO_REGISTER];
 /* Bitmap used for different calculations.  */
 static bitmap_head temp_bitmap;
 
+/* Registers accessed via subreg_p.  */
+static bitmap_head subreg_regs;
+
 typedef struct cand *cand_t;
 typedef const struct cand *const_cand_t;
 
@@ -383,30 +386,30 @@  operand_to_remat (rtx_insn *insn)
     return -1;
   /* First find a pseudo which can be rematerialized.  */
   for (reg = id->regs; reg != NULL; reg = reg->next)
-    /* True FRAME_POINTER_NEEDED might be because we can not follow
-       changing sp offsets, e.g. alloca is used.  If the insn contains
-       stack pointer in such case, we can not rematerialize it as we
-       can not know sp offset at a rematerialization place.  */
-    if (reg->regno == STACK_POINTER_REGNUM && frame_pointer_needed)
-      return -1;
-    else if (reg->type == OP_OUT && ! reg->subreg_p
-	     && find_regno_note (insn, REG_UNUSED, reg->regno) == NULL)
-      {
-	/* We permits only one spilled reg.  */
-	if (found_reg != NULL)
-	  return -1;
-	found_reg = reg;
-      }
-    /* IRA calculates conflicts separately for subregs of two words
-       pseudo.  Even if the pseudo lives, e.g. one its subreg can be
-       used lately, another subreg hard register can be already used
-       for something else.  In such case, it is not safe to
-       rematerialize the insn.  */
-    else if (reg->type == OP_IN && reg->subreg_p
-	     && reg->regno >= FIRST_PSEUDO_REGISTER
-	     && (GET_MODE_SIZE (PSEUDO_REGNO_MODE (reg->regno))
-		 == 2 * UNITS_PER_WORD))
-      return -1;
+    {
+      /* True FRAME_POINTER_NEEDED might be because we can not follow
+	 changing sp offsets, e.g. alloca is used.  If the insn contains
+	 stack pointer in such case, we can not rematerialize it as we
+	 can not know sp offset at a rematerialization place.  */
+      if (reg->regno == STACK_POINTER_REGNUM && frame_pointer_needed)
+	return -1;
+      else if (reg->type == OP_OUT && ! reg->subreg_p
+	       && find_regno_note (insn, REG_UNUSED, reg->regno) == NULL)
+	{
+	  /* We permits only one spilled reg.  */
+	  if (found_reg != NULL)
+	    return -1;
+	  found_reg = reg;
+        }
+      /* IRA calculates conflicts separately for subregs of two words
+	 pseudo.  Even if the pseudo lives, e.g. one its subreg can be
+	 used lately, another subreg hard register can be already used
+	 for something else.  In such case, it is not safe to
+	 rematerialize the insn.  */
+      if (reg->regno >= FIRST_PSEUDO_REGISTER
+	  && bitmap_bit_p (&subreg_regs, reg->regno))
+	return -1;
+    }
   if (found_reg == NULL)
     return -1;
   if (found_reg->regno < FIRST_PSEUDO_REGISTER)
@@ -631,6 +634,9 @@  dump_candidates_and_remat_bb_data (void)
       lra_dump_bitmap_with_title ("avout cands in BB",
 				  &get_remat_bb_data (bb)->avout_cands, bb->index);
     }
+  fprintf (lra_dump_file, "subreg regs:");
+  dump_regset (&subreg_regs, lra_dump_file);
+  putc ('\n', lra_dump_file);
 }
 
 /* Free all BB data.  */
@@ -655,21 +661,24 @@  finish_remat_bb_data (void)
 
 
 
-/* Update changed_regs and dead_regs of BB from INSN.  */
+/* Update changed_regs, dead_regs, subreg_regs of BB from INSN.  */
 static void
 set_bb_regs (basic_block bb, rtx_insn *insn)
 {
   lra_insn_recog_data_t id = lra_get_insn_recog_data (insn);
+  remat_bb_data_t bb_info = get_remat_bb_data (bb);
   struct lra_insn_reg *reg;
 
   for (reg = id->regs; reg != NULL; reg = reg->next)
-    if (reg->type != OP_IN)
-      bitmap_set_bit (&get_remat_bb_data (bb)->changed_regs, reg->regno);
-    else
-      {
-	if (find_regno_note (insn, REG_DEAD, (unsigned) reg->regno) != NULL)
-	  bitmap_set_bit (&get_remat_bb_data (bb)->dead_regs, reg->regno);
-      }
+    {
+      unsigned regno = reg->regno;
+      if (reg->type != OP_IN)
+        bitmap_set_bit (&bb_info->changed_regs, regno);
+      else if (find_regno_note (insn, REG_DEAD, regno) != NULL)
+	bitmap_set_bit (&bb_info->dead_regs, regno);
+      if (regno >= FIRST_PSEUDO_REGISTER && reg->subreg_p)
+	bitmap_set_bit (&subreg_regs, regno);
+    }
   if (CALL_P (insn))
     for (int i = 0; i < call_used_regs_arr_len; i++)
       bitmap_set_bit (&get_remat_bb_data (bb)->dead_regs,
@@ -1284,10 +1293,11 @@  lra_remat (void)
     if (call_used_regs[i])
       call_used_regs_arr[call_used_regs_arr_len++] = i;
   initiate_cand_table ();
-  create_cands ();
   create_remat_bb_data ();
   bitmap_initialize (&temp_bitmap, &reg_obstack);
+  bitmap_initialize (&subreg_regs, &reg_obstack);
   calculate_local_reg_remat_bb_data ();
+  create_cands ();
   calculate_livein_cands ();
   calculate_gen_cands ();
   bitmap_initialize (&all_blocks, &reg_obstack);
@@ -1298,6 +1308,7 @@  lra_remat (void)
   result = do_remat ();
   all_cands.release ();
   bitmap_clear (&temp_bitmap);
+  bitmap_clear (&subreg_regs);
   finish_remat_bb_data ();
   finish_cand_table ();
   bitmap_clear (&all_blocks);
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr69447.c b/gcc/testsuite/gcc.c-torture/execute/pr69447.c
new file mode 100644
index 0000000..b6d8591
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr69447.c
@@ -0,0 +1,26 @@ 
+typedef unsigned char u8;
+typedef unsigned short u16;
+typedef unsigned int u32;
+typedef unsigned long long u64;
+
+u64 __attribute__((noinline, noclone))
+foo(u8 u8_0, u16 u16_0, u64 u64_0, u8 u8_1, u16 u16_1, u64 u64_1, u64 u64_2, u8 u8_3, u64 u64_3)
+{
+	u64_1 *= 0x7730;
+	u64_3 *= u64_3;
+	u16_1 |= u64_3;
+	u64_3 -= 2;
+	u8_3 /= u64_2;
+	u8_0 |= 3;
+	u64_3 %= u8_0;
+	u8_0 -= 1;
+	return u8_0 + u16_0 + u64_0 + u8_1 + u16_1 + u64_1 + u8_3 + u64_3;
+}
+
+int main()
+{
+	unsigned x = foo(1, 1, 1, 1, 1, 1, 1, 1, 1);
+	if (x != 0x7737)
+		__builtin_abort();
+	return 0;
+}