patch to fix PR55433

Submitted by Vladimir Makarov on Jan. 18, 2013, 6:25 p.m.

Details

Message ID 50F993AD.5000509@redhat.com
State New
Headers show

Commit Message

Vladimir Makarov Jan. 18, 2013, 6:25 p.m.
The following patch fixes

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55433

LRA went into infinite reload loop when secondary memory was needed.

To fix the problem, two changes are done:
    o a typo was fixed in inheritance which prevented rejecting 
inheritances requiring secondary memory reloads
    o still a spilled pseudo was reassigned to a register in LRA 
resulting in secondary memory reloads.  it is desirable to prevent it 
too but it is hard to do at this stage.
    o in any case we should have prevented infinite reload loop and it 
was achieved by reusing an original insn for secondary memory reloads.  
It is useful in case of the PR as the original insn is not a simple move 
(it contains also a pseudo clobber).

The last change is not trivial.  I worked on it for a few day.  I guess 
there is a tiny possibility that it breaks something but I myself did 
not any problems with the change.

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

Committed as rev. 195302.

2013-01-18  Vladimir Makarov  <vmakarov@redhat.com>

         PR target/55433
         * lra-constraints.c (curr_insn_transform): Reuse original insn for
         secondary memory move.
         (inherit_reload_reg): Use rclass instead of cl for
         check_secondary_memory_needed_p.

2013-01-18  Vladimir Makarov  <vmakarov@redhat.com>

         PR target/55433
         * gcc.target/i386/pr55433.c: New.

Comments

Steven Bosscher Jan. 18, 2013, 6:34 p.m.
On Fri, Jan 18, 2013 at 7:25 PM, Vladimir Makarov wrote:
>
>         PR target/55433
>         * lra-constraints.c (curr_insn_transform): Reuse original insn for
>         secondary memory move.
>         (inherit_reload_reg): Use rclass instead of cl for
>         check_secondary_memory_needed_p.

Can you put this on the LRA branch also, please?

Thanks,

Ciao!
Steven
Vladimir Makarov Jan. 18, 2013, 11:11 p.m.
On 13-01-18 1:34 PM, Steven Bosscher wrote:
> On Fri, Jan 18, 2013 at 7:25 PM, Vladimir Makarov wrote:
>>          PR target/55433
>>          * lra-constraints.c (curr_insn_transform): Reuse original insn for
>>          secondary memory move.
>>          (inherit_reload_reg): Use rclass instead of cl for
>>          check_secondary_memory_needed_p.
> Can you put this on the LRA branch also, please?
>
>
I merged trunk with this patch into LRA branch instead.

Patch hide | download patch | download mbox

Index: lra-constraints.c
===================================================================
--- lra-constraints.c	(revision 195301)
+++ lra-constraints.c	(working copy)
@@ -2791,30 +2791,42 @@  curr_insn_transform (void)
 
   if (use_sec_mem_p)
     {
-      rtx new_reg, set, src, dest;
-      enum machine_mode sec_mode;
+      rtx new_reg, src, dest, rld, rld_subst;
+      enum machine_mode sec_mode, rld_mode;
 
       lra_assert (sec_mem_p);
-      set = single_set (curr_insn);
-      lra_assert (set != NULL_RTX && ! side_effects_p (set));
-      dest = SET_DEST (set);
-      src = SET_SRC (set);
+      lra_assert (curr_static_id->operand[0].type == OP_OUT
+		  && curr_static_id->operand[1].type == OP_IN);
+      dest = *curr_id->operand_loc[0];
+      src = *curr_id->operand_loc[1];
+      rld = (GET_MODE_SIZE (GET_MODE (dest)) <= GET_MODE_SIZE (GET_MODE (src))
+	     ? dest : src);
+      rld_mode = GET_MODE (rld);
 #ifdef SECONDARY_MEMORY_NEEDED_MODE
-      sec_mode = SECONDARY_MEMORY_NEEDED_MODE (GET_MODE (src));
+      sec_mode = SECONDARY_MEMORY_NEEDED_MODE (rld_mode);
 #else
-      sec_mode = GET_MODE (src);
+      sec_mode = rld_mode;
 #endif
       new_reg = lra_create_new_reg (sec_mode, NULL_RTX,
 				    NO_REGS, "secondary");
       /* If the mode is changed, it should be wider.  */
-      lra_assert (GET_MODE_SIZE (GET_MODE (new_reg))
-		  >= GET_MODE_SIZE (GET_MODE (src)));
-      after = emit_spill_move (false, new_reg, dest);
-      lra_process_new_insns (curr_insn, NULL_RTX, after,
-			     "Inserting the sec. move");
-      before = emit_spill_move (true, new_reg, src);
-      lra_process_new_insns (curr_insn, before, NULL_RTX, "Changing on");
-      lra_set_insn_deleted (curr_insn);
+      lra_assert (GET_MODE_SIZE (sec_mode) >= GET_MODE_SIZE (rld_mode));
+      rld_subst = (sec_mode == rld_mode ? new_reg : gen_lowpart_SUBREG (rld_mode, new_reg));
+      if (dest == rld)
+	{
+	  *curr_id->operand_loc[0] = rld_subst;
+	  after = emit_spill_move (false, new_reg, dest);
+	  lra_process_new_insns (curr_insn, NULL_RTX, after,
+				 "Inserting the sec. move");
+	}
+      else
+	{
+	  *curr_id->operand_loc[1] = rld_subst;
+	  before = emit_spill_move (true, new_reg, src);
+	  lra_process_new_insns (curr_insn, before, NULL_RTX,
+				 "Inserting the sec. move");
+	}
+      lra_update_insn_regno_info (curr_insn);
       return true;
     }
 #endif
@@ -3801,7 +3813,7 @@  inherit_reload_reg (bool def_p, int orig
 
       rclass = cl;
     }
-  if (check_secondary_memory_needed_p (cl, next_usage_insns))
+  if (check_secondary_memory_needed_p (rclass, next_usage_insns))
     {
       /* Reject inheritance resulting in secondary memory moves.
 	 Otherwise, there is a danger in LRA cycling.  Also such
@@ -3820,7 +3832,7 @@  inherit_reload_reg (bool def_p, int orig
 		   "    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]);
+		   original_regno, reg_class_names[rclass]);
 	  fprintf (lra_dump_file,
 		   "    >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
 	}
Index: testsuite/gcc.target/i386/pr55433.c
===================================================================
--- testsuite/gcc.target/i386/pr55433.c	(revision 0)
+++ testsuite/gcc.target/i386/pr55433.c	(working copy)
@@ -0,0 +1,12 @@ 
+/* { dg-do compile {target { *-*-darwin* } } } */
+/* { dg-options "-O1 -m32" } */
+
+typedef unsigned long long tick_t;
+extern int foo(void);
+extern tick_t tick(void);
+double test(void) {
+  struct { tick_t ticks; } st;
+  st.ticks = tick();
+  foo();
+  return (double)st.ticks;
+}