diff mbox

Reload codegen improvement

Message ID 52CC29DF.7020701@codesourcery.com
State New
Headers show

Commit Message

Bernd Schmidt Jan. 7, 2014, 4:22 p.m. UTC
This fixes a problem identified by Chung-Lin. Once reload is done, all 
equivalencing insns for pseudos that didn't get a hard reg but could be 
eliminated using their REG_EQUIV are deleted. However, we still can 
produce reloads and reload insns for them in certain cases, leading to 
unnecessary spilling. This patch corrects this by making sure we use 
identical tests when deciding whether to ignore an insn while reloading, 
and whether to delete it afterwards.

Bootstrapped and tested on x86_64-linux (with lra_p disabled). Chung-Lin 
says he's tested it as well, I think on arm (probably with something 4.8 
based). Will commit in a few days if no objections.


Bernd

Comments

Chung-Lin Tang Jan. 23, 2014, 9:44 a.m. UTC | #1
On 14/1/8 12:22 AM, Bernd Schmidt wrote:
> This fixes a problem identified by Chung-Lin. Once reload is done, all
> equivalencing insns for pseudos that didn't get a hard reg but could be
> eliminated using their REG_EQUIV are deleted. However, we still can
> produce reloads and reload insns for them in certain cases, leading to
> unnecessary spilling. This patch corrects this by making sure we use
> identical tests when deciding whether to ignore an insn while reloading,
> and whether to delete it afterwards.
> 
> Bootstrapped and tested on x86_64-linux (with lra_p disabled). Chung-Lin
> says he's tested it as well, I think on arm (probably with something 4.8
> based). Will commit in a few days if no objections.
> 
> 
> Bernd

Hi Bernd, this does not seem to be committed yet.

Thanks,
Chung-Lin
Bernd Schmidt Jan. 23, 2014, 1:05 p.m. UTC | #2
On 01/23/2014 10:44 AM, Chung-Lin Tang wrote:
> On 14/1/8 12:22 AM, Bernd Schmidt wrote:
>> This fixes a problem identified by Chung-Lin. Once reload is done, all
>> equivalencing insns for pseudos that didn't get a hard reg but could be
>> eliminated using their REG_EQUIV are deleted. However, we still can
>> produce reloads and reload insns for them in certain cases, leading to
>> unnecessary spilling. This patch corrects this by making sure we use
>> identical tests when deciding whether to ignore an insn while reloading,
>> and whether to delete it afterwards.
>>
>> Bootstrapped and tested on x86_64-linux (with lra_p disabled). Chung-Lin
>> says he's tested it as well, I think on arm (probably with something 4.8
>> based). Will commit in a few days if no objections.
>>
> Hi Bernd, this does not seem to be committed yet.

Yeah - I decided this probably ought to wait for stage 1.


Bernd
Bernd Schmidt May 21, 2014, 9:33 a.m. UTC | #3
On 01/07/2014 05:22 PM, Bernd Schmidt wrote:
> This fixes a problem identified by Chung-Lin. Once reload is done, all
> equivalencing insns for pseudos that didn't get a hard reg but could be
> eliminated using their REG_EQUIV are deleted. However, we still can
> produce reloads and reload insns for them in certain cases, leading to
> unnecessary spilling. This patch corrects this by making sure we use
> identical tests when deciding whether to ignore an insn while reloading,
> and whether to delete it afterwards.
>
> Bootstrapped and tested on x86_64-linux (with lra_p disabled). Chung-Lin
> says he's tested it as well, I think on arm (probably with something 4.8
> based). Will commit in a few days if no objections.

I decided to wait until after 4.9.  Now committed after testing on 
bfin-elf - the trick of disabling LRA on x86_64 no longer works, the 
target doesn't build with reload enabled.

(I saw one compare-debug randomly failing or passing, which happens with 
or without this change. It seems to be caused by something in IRA).


Bernd
diff mbox

Patch

Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c	(revision 206316)
+++ gcc/reload1.c	(working copy)
@@ -686,6 +686,65 @@  static int failure;
 /* Temporary array of pseudo-register number.  */
 static int *temp_pseudo_reg_arr;
 
+/* If a pseudo has no hard reg, delete the insns that made the equivalence.
+   If that insn didn't set the register (i.e., it copied the register to
+   memory), just delete that insn instead of the equivalencing insn plus
+   anything now dead.  If we call delete_dead_insn on that insn, we may
+   delete the insn that actually sets the register if the register dies
+   there and that is incorrect.  */
+static void
+remove_init_insns ()
+{
+  for (int i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
+    {
+      if (reg_renumber[i] < 0 && reg_equiv_init (i) != 0)
+	{
+	  rtx list;
+	  for (list = reg_equiv_init (i); list; list = XEXP (list, 1))
+	    {
+	      rtx equiv_insn = XEXP (list, 0);
+
+	      /* If we already deleted the insn or if it may trap, we can't
+		 delete it.  The latter case shouldn't happen, but can
+		 if an insn has a variable address, gets a REG_EH_REGION
+		 note added to it, and then gets converted into a load
+		 from a constant address.  */
+	      if (NOTE_P (equiv_insn)
+		  || can_throw_internal (equiv_insn))
+		;
+	      else if (reg_set_p (regno_reg_rtx[i], PATTERN (equiv_insn)))
+		delete_dead_insn (equiv_insn);
+	      else
+		SET_INSN_DELETED (equiv_insn);
+	    }
+	}
+    }
+}
+
+/* Return true if remove_init_insns will delete INSN.  */
+static bool
+will_delete_init_insn_p (rtx insn)
+{
+  rtx set = single_set (insn);
+  if (!set || !REG_P (SET_DEST (set)))
+    return false;
+  unsigned regno = REGNO (SET_DEST (set));
+
+  if (can_throw_internal (insn))
+    return false;
+
+  if (regno < FIRST_PSEUDO_REGISTER || reg_renumber[regno] >= 0)
+    return false;
+  
+  for (rtx list = reg_equiv_init (regno); list; list = XEXP (list, 1))
+    {
+      rtx equiv_insn = XEXP (list, 0);
+      if (equiv_insn == insn)
+	return true;
+    }
+  return false;
+}
+
 /* Main entry point for the reload pass.
 
    FIRST is the first insn of the function being compiled.
@@ -993,37 +1052,7 @@  reload (rtx first, int global)
       if (ep->can_eliminate)
 	mark_elimination (ep->from, ep->to);
 
-  /* If a pseudo has no hard reg, delete the insns that made the equivalence.
-     If that insn didn't set the register (i.e., it copied the register to
-     memory), just delete that insn instead of the equivalencing insn plus
-     anything now dead.  If we call delete_dead_insn on that insn, we may
-     delete the insn that actually sets the register if the register dies
-     there and that is incorrect.  */
-
-  for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
-    {
-      if (reg_renumber[i] < 0 && reg_equiv_init (i) != 0)
-	{
-	  rtx list;
-	  for (list = reg_equiv_init (i); list; list = XEXP (list, 1))
-	    {
-	      rtx equiv_insn = XEXP (list, 0);
-
-	      /* If we already deleted the insn or if it may trap, we can't
-		 delete it.  The latter case shouldn't happen, but can
-		 if an insn has a variable address, gets a REG_EH_REGION
-		 note added to it, and then gets converted into a load
-		 from a constant address.  */
-	      if (NOTE_P (equiv_insn)
-		  || can_throw_internal (equiv_insn))
-		;
-	      else if (reg_set_p (regno_reg_rtx[i], PATTERN (equiv_insn)))
-		delete_dead_insn (equiv_insn);
-	      else
-		SET_INSN_DELETED (equiv_insn);
-	    }
-	}
-    }
+  remove_init_insns ();
 
   /* Use the reload registers where necessary
      by generating move instructions to move the must-be-register
@@ -1484,14 +1513,9 @@  calculate_needs_all_insns (int global)
 	  rtx old_notes = REG_NOTES (insn);
 	  int did_elimination = 0;
 	  int operands_changed = 0;
-	  rtx set = single_set (insn);
 
 	  /* Skip insns that only set an equivalence.  */
-	  if (set && REG_P (SET_DEST (set))
-	      && reg_renumber[REGNO (SET_DEST (set))] < 0
-	      && (reg_equiv_constant (REGNO (SET_DEST (set)))
-		  || (reg_equiv_invariant (REGNO (SET_DEST (set)))))
-		      && reg_equiv_init (REGNO (SET_DEST (set))))
+	  if (will_delete_init_insn_p (insn))
 	    continue;
 
 	  /* If needed, eliminate any eliminable registers.  */
@@ -4586,6 +4610,9 @@  reload_as_needed (int live_known)
       rtx old_prev = PREV_INSN (insn);
 #endif
 
+      if (will_delete_init_insn_p (insn))
+	continue;
+
       /* If we pass a label, copy the offsets from the label information
 	 into the current offsets of each elimination.  */
       if (LABEL_P (insn))