diff mbox

patch to fix PR55106

Message ID 508DD119.5040509@redhat.com
State New
Headers show

Commit Message

Vladimir Makarov Oct. 29, 2012, 12:43 a.m. UTC 
The following patch fixes PR55106.  A value in GENERAL_REGS is 
inherited into a move with destination pseudo of SSE_REGS.  It results 
into secondary move for which inheritance is tried again an again.  It 
means cycling LRA passes.

   The patch was successfully bootstrapped on x86/x86-64.

   Committed as rev. 192904.

2012-10-28  Vladimir Makarov  <vmakarov@redhat.com>

     PR rtl-optimization/55106
     * lra-constraints.c (skip_usage_debug_insns): New function.
     (check_secondary_memory_needed_p): Ditto.
     (inherit_reload_reg): Use the new functions.  Improve debug
     output.

Comments

H.J. Lu Oct. 29, 2012, 12:49 a.m. UTC | #1 
On Sun, Oct 28, 2012 at 5:43 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>   The following patch fixes PR55106.  A value in GENERAL_REGS is inherited
> into a move with destination pseudo of SSE_REGS.  It results into secondary
> move for which inheritance is tried again an again.  It means cycling LRA
> passes.
>
>   The patch was successfully bootstrapped on x86/x86-64.
>
>   Committed as rev. 192904.
>
> 2012-10-28  Vladimir Makarov  <vmakarov@redhat.com>
>
>     PR rtl-optimization/55106
>     * lra-constraints.c (skip_usage_debug_insns): New function.
>     (check_secondary_memory_needed_p): Ditto.
>     (inherit_reload_reg): Use the new functions.  Improve debug
>     output.
>

Please add the testcase at

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55106#c1

Thanks.
diff mbox

Patch

Index: lra-constraints.c
===================================================================
--- lra-constraints.c	(revision 192897)
+++ lra-constraints.c	(working copy)
@@ -3573,6 +3573,48 @@  substitute_pseudo (rtx *loc, int old_reg
   return result;
 }
 
+/* Return first non-debug insn in list USAGE_INSNS.  */
+static rtx
+skip_usage_debug_insns (rtx usage_insns)
+{
+  rtx insn;
+
+  /* Skip debug insns.  */
+  for (insn = usage_insns;
+       insn != NULL_RTX && GET_CODE (insn) == INSN_LIST;
+       insn = XEXP (insn, 1))
+    ;
+  return insn;
+}
+
+/* Return true if we need secondary memory moves for insn in
+   USAGE_INSNS after inserting inherited pseudo of class INHER_CL
+   into the insn.  */
+static bool
+check_secondary_memory_needed_p (enum reg_class inher_cl, rtx usage_insns)
+{
+#ifndef SECONDARY_MEMORY_NEEDED
+  return false;
+#else
+  rtx insn, set, dest;
+  enum reg_class cl;
+
+  if (inher_cl == ALL_REGS
+      || (insn = skip_usage_debug_insns (usage_insns)) == NULL_RTX)
+    return false;
+  lra_assert (INSN_P (insn));
+  if ((set = single_set (insn)) == NULL_RTX || ! REG_P (SET_DEST (set)))
+    return false;
+  dest = SET_DEST (set);
+  if (! REG_P (dest))
+    return false;
+  lra_assert (inher_cl != NO_REGS);
+  cl = get_reg_class (REGNO (dest));
+  return (cl != NO_REGS && cl != ALL_REGS
+	  && SECONDARY_MEMORY_NEEDED (inher_cl, cl, GET_MODE (dest)));
+#endif
+}
+
 /* Registers involved in inheritance/split in the current EBB
    (inheritance/split pseudos and original registers).	*/
 static bitmap_head check_only_regs;
@@ -3610,18 +3652,18 @@  inherit_reload_reg (bool def_p, int orig
   lra_assert (! usage_insns[original_regno].after_p);
   if (lra_dump_file != NULL)
     fprintf (lra_dump_file,
-	     "	  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
+	     "    <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
   if (! ira_reg_classes_intersect_p[cl][rclass])
     {
       if (lra_dump_file != NULL)
 	{
 	  fprintf (lra_dump_file,
-		   "	Rejecting inheritance for %d "
+		   "    Rejecting inheritance for %d "
 		   "because of disjoint classes %s and %s\n",
 		   original_regno, reg_class_names[cl],
 		   reg_class_names[rclass]);
 	  fprintf (lra_dump_file,
-		   "	>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
+		   "    >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
 	}
       return false;
     }
@@ -3638,6 +3680,31 @@  inherit_reload_reg (bool def_p, int orig
       
       rclass = cl;
     }
+  if (check_secondary_memory_needed_p (cl, next_usage_insns))
+    {
+      /* Reject inheritance resulting in secondary memory moves.
+	 Otherwise, there is a danger in LRA cycling.  Also such
+	 transformation will be unprofitable.  */
+      if (lra_dump_file != NULL)
+	{
+	  rtx insn = skip_usage_debug_insns (next_usage_insns);
+	  rtx set = single_set (insn);
+
+	  lra_assert (set != NULL_RTX);
+
+	  rtx dest = SET_DEST (set);
+
+	  lra_assert (REG_P (dest));
+	  fprintf (lra_dump_file,
+		   "    Rejecting inheritance for insn %d(%s)<-%d(%s) "
+		   "as secondary mem is needed\n",
+		   REGNO (dest), reg_class_names[get_reg_class (REGNO (dest))],
+		   original_regno, reg_class_names[cl]);
+	  fprintf (lra_dump_file,
+		   "    >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
+	}
+      return false;
+    }
   new_reg = lra_create_new_reg (GET_MODE (original_reg), original_reg,
 				rclass, "inheritance");
   start_sequence ();
@@ -3652,7 +3719,7 @@  inherit_reload_reg (bool def_p, int orig
       if (lra_dump_file != NULL)
 	{
 	  fprintf (lra_dump_file,
-		   "	Rejecting inheritance %d->%d "
+		   "    Rejecting inheritance %d->%d "
 		   "as it results in 2 or more insns:\n",
 		   original_regno, REGNO (new_reg));
 	  debug_rtl_slim (lra_dump_file, new_insns, NULL_RTX, -1, 0);
@@ -3667,7 +3734,7 @@  inherit_reload_reg (bool def_p, int orig
     /* We now have a new usage insn for original regno.  */
     setup_next_usage_insn (original_regno, new_insns, reloads_num, false);
   if (lra_dump_file != NULL)
-    fprintf (lra_dump_file, "	 Original reg change %d->%d (bb%d):\n",
+    fprintf (lra_dump_file, "    Original reg change %d->%d (bb%d):\n",
 	     original_regno, REGNO (new_reg), BLOCK_FOR_INSN (insn)->index);
   lra_reg_info[REGNO (new_reg)].restore_regno = original_regno;
   bitmap_set_bit (&check_only_regs, REGNO (new_reg));