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]

Re: reload inheritance error breaks openssl on s390


On Aug  9, 2006, Ian Lance Taylor <iant@google.com> wrote:

>> PR target/28146
>> * reload.h (reg_equiv_lra_tweak): New declaration.
>> * reload1.c (reg_equiv_lra_tweak): New definition.
>> (reload): Initialize it.
>> (delete_output_reload): Use it.
>> * reload.c (find_reloads_address): Set it if
>> LEGITIMIZE_RELOAD_ADDRESS changes the address.

> This patch is the right sort of thing, but I don't understand how it
> actually works.  You only store a value in reg_equiv_lra_tweak if
> REGNO >= 0, but I don't see any execution path which leads to REGNO >=
> 0 when LEGITIMIZE_RELOAD_ADDRESS is called.  What am I missing?

You're missing the brain-deadness that I experienced at the time I
wrote it :-(

I'd actually got away from the problem for too long, and when I got
back I found this simple, easy and wrong approach to address the
problem, that I was missing before :-), and random fluctuations in the
patterns that made to the reload pass managed to hide the problem such
that I no longer observed it.  Doh!

This one does a better job, and I've actually tested it on the same
baseline tree where I'd first experienced the problem.  Does this look
better for you too? :-)

I'm currently bootstrapping it on amd64-linux-gnu.  If that works,
I'll give it a round of testing on a wider range of machines and then
formally submit it for inclusion.

for gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR target/28146
	* reload.h (reg_equiv_alt_mem_list): New declaration.
	* reload1.c (reg_equiv_alt_mem_list): New definition.
	(reload): Initialize it and release it.
	(delete_output_reload): Use it.
	* reload.c (push_reg_equiv_alt_mem): New function.
	(find_reloads_toplev): Call it.
	(find_reloads_address, find_reloads_address_1): Likewise.
	(find_reloads_subreg_address): Likewise.

Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c.orig	2006-08-15 07:01:19.000000000 -0300
+++ gcc/reload1.c	2006-08-16 05:46:18.000000000 -0300
@@ -123,6 +123,10 @@ rtx *reg_equiv_address;
    or zero if pseudo reg N is not equivalent to a memory slot.  */
 rtx *reg_equiv_mem;
 
+/* Element N is an EXPR_LIST of REG_EQUIVs containing MEMs with
+   alternate representations of the location of pseudo reg N.  */
+rtx *reg_equiv_alt_mem_list;
+
 /* Widest width in which each pseudo reg is referred to (via subreg).  */
 static unsigned int *reg_max_ref_width;
 
@@ -703,6 +707,7 @@ reload (rtx first, int global)
   reg_equiv_constant = XCNEWVEC (rtx, max_regno);
   reg_equiv_invariant = XCNEWVEC (rtx, max_regno);
   reg_equiv_mem = XCNEWVEC (rtx, max_regno);
+  reg_equiv_alt_mem_list = XCNEWVEC (rtx, max_regno);
   reg_equiv_address = XCNEWVEC (rtx, max_regno);
   reg_max_ref_width = XCNEWVEC (unsigned int, max_regno);
   reg_old_renumber = XCNEWVEC (short, max_regno);
@@ -1260,6 +1265,11 @@ reload (rtx first, int global)
   if (offsets_at)
     free (offsets_at);
 
+  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+    if (reg_equiv_alt_mem_list[i])
+      free_EXPR_LIST_list (&reg_equiv_alt_mem_list[i]);
+  free (reg_equiv_alt_mem_list);
+
   free (reg_equiv_mem);
   reg_equiv_init = 0;
   free (reg_equiv_address);
@@ -7849,6 +7859,11 @@ delete_output_reload (rtx insn, int j, i
     n_occurrences += count_occurrences (PATTERN (insn),
 					eliminate_regs (substed, 0,
 							NULL_RTX), 0);
+  for (i1 = reg_equiv_alt_mem_list [REGNO (reg)]; i1; i1 = XEXP (i1, 1))
+    {
+      gcc_assert (!rtx_equal_p (XEXP (i1, 0), substed));
+      n_occurrences += count_occurrences (PATTERN (insn), XEXP (i1, 0), 0);
+    }
   if (n_occurrences > n_inherited)
     return;
 
Index: gcc/reload.c
===================================================================
--- gcc/reload.c.orig	2006-08-15 07:01:19.000000000 -0300
+++ gcc/reload.c	2006-08-16 05:45:02.000000000 -0300
@@ -283,6 +283,26 @@ static int find_inc_amount (rtx, rtx);
 static int refers_to_mem_for_reload_p (rtx);
 static int refers_to_regno_for_reload_p (unsigned int, unsigned int,
 					 rtx, rtx *);
+
+/* Add NEW to reg_equiv_alt_mem_list[REGNO] if it's different from
+   ORIG and not present in the list yet.  */
+
+static inline void
+push_reg_equiv_alt_mem (int regno, rtx new, rtx orig)
+{
+  rtx it;
+
+  if (new == orig)
+    return;
+
+  for (it = reg_equiv_alt_mem_list [regno]; it; it = XEXP (it, 1))
+    if (rtx_equal_p (XEXP (it, 0), new))
+      return;
+
+  reg_equiv_alt_mem_list [regno]
+    = alloc_EXPR_LIST (REG_EQUIV, new,
+		       reg_equiv_alt_mem_list [regno]);
+}
 
 /* Determine if any secondary reloads are needed for loading (if IN_P is
    nonzero) or storing (if IN_P is zero) X to or from a reload register of
@@ -4553,6 +4573,7 @@ find_reloads_toplev (rtx x, int opnum, e
 	      x = mem;
 	      i = find_reloads_address (GET_MODE (x), &x, XEXP (x, 0), &XEXP (x, 0),
 					opnum, type, ind_levels, insn);
+	      push_reg_equiv_alt_mem (regno, x, mem);
 	      if (address_reloaded)
 		*address_reloaded = i;
 	    }
@@ -4761,9 +4782,12 @@ find_reloads_address (enum machine_mode 
 	      tem = make_memloc (ad, regno);
 	      if (! strict_memory_address_p (GET_MODE (tem), XEXP (tem, 0)))
 		{
+		  rtx orig = tem;
+
 		  find_reloads_address (GET_MODE (tem), &tem, XEXP (tem, 0),
 					&XEXP (tem, 0), opnum,
 					ADDR_TYPE (type), ind_levels, insn);
+		  push_reg_equiv_alt_mem (regno, tem, orig);
 		}
 	      /* We can avoid a reload if the register's equivalent memory
 		 expression is valid as an indirect memory address.
@@ -5545,6 +5569,8 @@ find_reloads_address_1 (enum machine_mod
 	    if (reg_equiv_address[regno]
 		|| ! rtx_equal_p (tem, reg_equiv_mem[regno]))
 	      {
+		rtx orig = tem;
+
 		/* First reload the memory location's address.
 		    We can't use ADDR_TYPE (type) here, because we need to
 		    write back the value after reading it, hence we actually
@@ -5554,6 +5580,8 @@ find_reloads_address_1 (enum machine_mod
 				      RELOAD_OTHER,
 				      ind_levels, insn);
 
+		push_reg_equiv_alt_mem (regno, tem, orig);
+
 		/* Then reload the memory location into a base
 		   register.  */
 		reloadnum = push_reload (tem, tem, &XEXP (x, 0),
@@ -5609,6 +5637,8 @@ find_reloads_address_1 (enum machine_mod
 	      if (reg_equiv_address[regno]
 		  || ! rtx_equal_p (tem, reg_equiv_mem[regno]))
 		{
+		  rtx orig = tem;
+
 		  /* First reload the memory location's address.
 		     We can't use ADDR_TYPE (type) here, because we need to
 		     write back the value after reading it, hence we actually
@@ -5616,6 +5646,7 @@ find_reloads_address_1 (enum machine_mod
 		  find_reloads_address (GET_MODE (tem), &tem, XEXP (tem, 0),
 					&XEXP (tem, 0), opnum, type,
 					ind_levels, insn);
+		  push_reg_equiv_alt_mem (regno, tem, orig);
 		  /* Put this inside a new increment-expression.  */
 		  x = gen_rtx_fmt_e (GET_CODE (x), GET_MODE (x), tem);
 		  /* Proceed to reload that, as if it contained a register.  */
@@ -5806,6 +5837,7 @@ find_reloads_address_1 (enum machine_mod
 		find_reloads_address (GET_MODE (x), &x, XEXP (x, 0),
 				      &XEXP (x, 0), opnum, ADDR_TYPE (type),
 				      ind_levels, insn);
+		push_reg_equiv_alt_mem (regno, x, tem);
 	      }
 	  }
 
@@ -5993,6 +6025,7 @@ find_reloads_subreg_address (rtx x, int 
 	      unsigned outer_size = GET_MODE_SIZE (GET_MODE (x));
 	      unsigned inner_size = GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)));
 	      int offset;
+	      rtx orig = tem;
 
 	      /* For big-endian paradoxical subregs, SUBREG_BYTE does not
 		 hold the correct (negative) byte offset.  */
@@ -6028,6 +6061,9 @@ find_reloads_subreg_address (rtx x, int 
 	      find_reloads_address (GET_MODE (tem), &tem, XEXP (tem, 0),
 				    &XEXP (tem, 0), opnum, type,
 				    ind_levels, insn);
+	      /* ??? Do we need to handle nonzero offsets somehow?  */
+	      if (!offset)
+		push_reg_equiv_alt_mem (regno, tem, orig);
 
 	      /* If this is not a toplevel operand, find_reloads doesn't see
 		 this substitution.  We have to emit a USE of the pseudo so
Index: gcc/reload.h
===================================================================
--- gcc/reload.h.orig	2006-08-15 07:01:19.000000000 -0300
+++ gcc/reload.h	2006-08-16 05:44:44.000000000 -0300
@@ -161,6 +161,7 @@ extern rtx *reg_equiv_invariant;
 extern rtx *reg_equiv_memory_loc;
 extern rtx *reg_equiv_address;
 extern rtx *reg_equiv_mem;
+extern rtx *reg_equiv_alt_mem_list;
 
 /* Element N is the list of insns that initialized reg N from its equivalent
    constant or memory slot.  */
-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
Secretary for FSF Latin America        http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

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