alias fix for PR58685

[Bernd Schmidt <bernds at codesourcery dot com>]

The sequence of events here can be summarized as "shrink-wrapping causes
the i386 backend to do something that confuses alias analysis". The
miscompilation is that two instructions are swapped by the scheduler
when they shouldn't be, due to an incorrect reg_base_value.

The miscompiled function has the following parts:
 * a loop which uses %rdi as an incoming argument register
 * following that, a decrement of the stack pointer (shrink-wrapped
   to this point rather than the start of the function)
 * the rest of the function which has one set of %rdi to
   (symbol_ref LC0).

The argument register %rdi is dead by the point where we decrement the
stack pointer. The i386 backend splits the sub into a sequence of two
insns, a clobber of %rdi and a push of it (which register is chosen
appears to be somewhat random).

When called from the scheduler, init_alias_analysis incorrectly deduces
that %rdi has a base value of (symbol_ref LC0). Although it tries to
track which registers are argument registers that may hold a pointer,
this information is lost when we encounter the clobber. The main part of
the following patch is to modify that piece of code to also set reg_seen
for argument registers when encountering a clobber.

There are other problems in this area, one of which showed up while
testing an earlier patch. i386/pr57003.c demonstrates that return values
of FUNCTION_ARG_REGNO are not constant across the whole compilation;
they are affected by the ms_abi attribute. This necessitates changing
from a static_reg_base_value array to a per-function one. Once that is
done, it's better to look at DECL_ARGUMENTS in a similar way to what
combine.c does to identify the arguments of the specific function we're
compiling rather than using the less specific FUNCTION_ARG_REGNO.
Lastly, I modified a test for Pmode to use the valid_pointer_mode hook
which should be a little more correct.

Bootstrapped and tested on x86_64-linux. Ok?


Bernd

	PR rtl-optimization/58685
	* alias.c (static_reg_base_value): Delete macro.
	(reg_base_value_initial): New macro to replace it.  All uses changed.
	(record_set): For a clobber of an arg reg, set reg_seen.
	(setup_initial_base_values): Renamed from init_alias_target and made
	static.  Check the DECL_INCOMING_RTL of the DECL_ARGUMENTS.  Record
	that the array was set up for the current function.
	(init_alias_analysis): Call setup_initial_base_values if necessary.
	* function.h (struct emit_status): Add fields initial_reg_base_value
	and initial_reg_base_value_setup.
	* rtl.h (struct target_rtl): Remove x_static_reg_base_value.
	(init_alias_target): Don't declare.
	* toplev.c (backend_init_target): Don't call it.

	PR rtl-optimization/58685
	* gcc.c-torture/execute/pr58685.c: New test.

diff --git a/gcc/alias.c b/gcc/alias.c
index a48bb51..b1189b7 100644
--- a/gcc/alias.c
+++ b/gcc/alias.c
@@ -229,8 +229,7 @@ static GTY((deletable)) vec<rtx, va_gc> *old_reg_base_value;
 #define UNIQUE_BASE_VALUE_FP	-3
 #define UNIQUE_BASE_VALUE_HFP	-4
 
-#define static_reg_base_value \
-  (this_target_rtl->x_static_reg_base_value)
+#define reg_base_value_initial (crtl->emit.initial_reg_base_value)
 
 #define REG_BASE_VALUE(X)					\
   (REGNO (X) < vec_safe_length (reg_base_value)			\
@@ -1306,6 +1305,20 @@ record_set (rtx dest, const_rtx set, void *data ATTRIBUTE_UNUSED)
 	 set).  */
       if (GET_CODE (set) == CLOBBER)
 	{
+	  if (new_reg_base_value[regno] != 0
+	      && regno != HARD_FRAME_POINTER_REGNUM
+	      && regno != FRAME_POINTER_REGNUM
+	      && regno != STACK_POINTER_REGNUM
+	      && regno != ARG_POINTER_REGNUM)
+	    {
+	      if (!bitmap_bit_p (reg_seen, regno))
+		{
+		  /* We have to make an exception here for an argument
+		     register, in case it is used before the clobber.  */
+		  gcc_assert (new_reg_base_value[regno] == arg_base_value);
+		  bitmap_set_bit (reg_seen, regno);
+		}
+	    }
 	  new_reg_base_value[regno] = 0;
 	  return;
 	}
@@ -1693,7 +1706,7 @@ find_base_term (rtx x)
 	return ret;
 
       if (cselib_sp_based_value_p (val))
-	return static_reg_base_value[STACK_POINTER_REGNUM];
+	return reg_base_value_initial[STACK_POINTER_REGNUM];
 
       f = val->locs;
       /* Temporarily reset val->locs to avoid infinite recursion.  */
@@ -1795,7 +1808,7 @@ bool
 may_be_sp_based_p (rtx x)
 {
   rtx base = find_base_term (x);
-  return !base || base == static_reg_base_value[STACK_POINTER_REGNUM];
+  return !base || base == reg_base_value_initial[STACK_POINTER_REGNUM];
 }
 
 /* Return 0 if the addresses X and Y are known to point to different
@@ -2809,34 +2822,34 @@ may_alias_p (const_rtx mem, const_rtx x)
   return rtx_refs_may_alias_p (x, mem, false);
 }
 
-void
-init_alias_target (void)
+static void
+setup_initial_base_values (void)
 {
-  int i;
-
   if (!arg_base_value)
     arg_base_value = gen_rtx_ADDRESS (VOIDmode, 0);
 
-  memset (static_reg_base_value, 0, sizeof static_reg_base_value);
+  memset (reg_base_value_initial, 0, sizeof reg_base_value_initial);
 
-  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
-    /* Check whether this register can hold an incoming pointer
-       argument.  FUNCTION_ARG_REGNO_P tests outgoing register
-       numbers, so translate if necessary due to register windows.  */
-    if (FUNCTION_ARG_REGNO_P (OUTGOING_REGNO (i))
-	&& HARD_REGNO_MODE_OK (i, Pmode))
-      static_reg_base_value[i] = arg_base_value;
+  for (tree arg = DECL_ARGUMENTS (current_function_decl); arg;
+       arg = DECL_CHAIN (arg))
+    {
+      rtx reg = DECL_INCOMING_RTL (arg);
+      if (REG_P (reg) && targetm.valid_pointer_mode (GET_MODE (reg)))
+	reg_base_value_initial[REGNO (reg)] = arg_base_value;
+    }
 
-  static_reg_base_value[STACK_POINTER_REGNUM]
+  reg_base_value_initial[STACK_POINTER_REGNUM]
     = unique_base_value (UNIQUE_BASE_VALUE_SP);
-  static_reg_base_value[ARG_POINTER_REGNUM]
+  reg_base_value_initial[ARG_POINTER_REGNUM]
     = unique_base_value (UNIQUE_BASE_VALUE_ARGP);
-  static_reg_base_value[FRAME_POINTER_REGNUM]
+  reg_base_value_initial[FRAME_POINTER_REGNUM]
     = unique_base_value (UNIQUE_BASE_VALUE_FP);
 #if !HARD_FRAME_POINTER_IS_FRAME_POINTER
-  static_reg_base_value[HARD_FRAME_POINTER_REGNUM]
+  reg_base_value_initial[HARD_FRAME_POINTER_REGNUM]
     = unique_base_value (UNIQUE_BASE_VALUE_HFP);
 #endif
+
+  crtl->emit.initial_reg_base_value_setup = true;
 }
 
 /* Set MEMORY_MODIFIED when X modifies DATA (that is assumed
@@ -2914,6 +2927,9 @@ init_alias_analysis (void)
 
   timevar_push (TV_ALIAS_ANALYSIS);
 
+  if (!crtl->emit.initial_reg_base_value_setup)
+    setup_initial_base_values ();
+  
   vec_safe_grow_cleared (reg_known_value, maxreg - FIRST_PSEUDO_REGISTER);
   reg_known_equiv_p = sbitmap_alloc (maxreg - FIRST_PSEUDO_REGISTER);
   bitmap_clear (reg_known_equiv_p);
@@ -2983,7 +2999,7 @@ init_alias_analysis (void)
 	 The address expression is VOIDmode for an argument and
 	 Pmode for other registers.  */
 
-      memcpy (new_reg_base_value, static_reg_base_value,
+      memcpy (new_reg_base_value, reg_base_value_initial,
 	      FIRST_PSEUDO_REGISTER * sizeof (rtx));
 
       /* Walk the insns adding values to the new_reg_base_value array.  */
diff --git a/gcc/function.h b/gcc/function.h
index d1f4ffc..5cd16a5 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -79,6 +79,13 @@ struct GTY(()) emit_status {
      for that pseudo (if REG_POINTER is set in x_regno_reg_rtx).
      Allocated in parallel with x_regno_reg_rtx.  */
   unsigned char * GTY((skip)) regno_pointer_align;
+
+  /* Set up by init_alias_analysis and reused for successive invocations for
+     the same function.  Contains information about whether a register can
+     be used to hold a pointer argument.  */
+  rtx initial_reg_base_value[FIRST_PSEUDO_REGISTER];
+  /* True if the preceding array has been initialized for this function.  */
+  bool initial_reg_base_value_setup;
 };
 
 
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 247a0d0..7c92e9d 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2296,10 +2296,6 @@ struct GTY(()) target_rtl {
   /* A sample (mem:M stack_pointer_rtx) rtx for each mode M.  */
   rtx x_top_of_stack[MAX_MACHINE_MODE];
 
-  /* Static hunks of RTL used by the aliasing code; these are treated
-     as persistent to avoid unnecessary RTL allocations.  */
-  rtx x_static_reg_base_value[FIRST_PSEUDO_REGISTER];
-
   /* The default memory attributes for each mode.  */
   struct mem_attrs *x_mode_mem_attrs[(int) MAX_MACHINE_MODE];
 };
@@ -2713,7 +2709,6 @@ extern int canon_anti_dependence (const_rtx, bool,
 				  const_rtx, enum machine_mode, rtx);
 extern int output_dependence (const_rtx, const_rtx);
 extern int may_alias_p (const_rtx, const_rtx);
-extern void init_alias_target (void);
 extern void init_alias_analysis (void);
 extern void end_alias_analysis (void);
 extern void vt_equate_reg_base_value (const_rtx, const_rtx);
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr58685.c b/gcc/testsuite/gcc.c-torture/execute/pr58685.c
new file mode 100644
index 0000000..36e770c
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr58685.c
@@ -0,0 +1,144 @@
+/* This is a fragile testcase, requiring a random choice of push register
+   from the x86 backend to fail, hence the multiple copies of an identical
+   function.  */
+extern void abort ();
+extern void exit (int);
+int a, b, c = 1, d, e;
+int *f = &e, *g = &e, *i;
+
+char *p;
+__attribute__((noinline,noclone)) void fail (char *f)
+{
+  abort ();
+}
+
+void
+fn1 ()
+{
+  if (a)
+    i = 0;
+}
+
+
+int
+fn3 (int *p1)
+{
+  for (b = 0; b >= 0; b--)
+    {
+      int *h = &e;
+      *h = 0;
+      if (*p1)
+	continue;
+      e = 1;
+    }
+  fn1 ();
+  c ? (void) 0 : fail ("");
+  return 0;
+}
+
+int
+fn3b (int *p1)
+{
+  for (b = 0; b >= 0; b--)
+    {
+      int *h = &e;
+      *h = 0;
+      if (*p1)
+	continue;
+      e = 1;
+    }
+  fn1 ();
+  c ? (void) 0 : fail ("");
+  return 0;
+}
+int
+fn3c (int *p1)
+{
+  for (b = 0; b >= 0; b--)
+    {
+      int *h = &e;
+      *h = 0;
+      if (*p1)
+	continue;
+      e = 1;
+    }
+  fn1 ();
+  c ? (void) 0 : fail ("");
+  return 0;
+}
+int
+fn3d (int *p1)
+{
+  for (b = 0; b >= 0; b--)
+    {
+      int *h = &e;
+      *h = 0;
+      if (*p1)
+	continue;
+      e = 1;
+    }
+  fn1 ();
+  c ? (void) 0 : fail ("");
+  return 0;
+}
+int
+fn3e (int *p1)
+{
+  for (b = 0; b >= 0; b--)
+    {
+      int *h = &e;
+      *h = 0;
+      if (*p1)
+	continue;
+      e = 1;
+    }
+  fn1 ();
+  c ? (void) 0 : fail ("");
+  return 0;
+}
+int
+fn3f (int *p1)
+{
+  for (b = 0; b >= 0; b--)
+    {
+      int *h = &e;
+      *h = 0;
+      if (*p1)
+	continue;
+      e = 1;
+    }
+  fn1 ();
+  c ? (void) 0 : fail ("");
+  return 0;
+}
+
+int
+main ()
+{
+  e = 1;
+  fn3 (g);
+  if (!*f)
+    abort ();
+  e = 1;
+  fn3b (g);
+  if (!*f)
+    abort ();
+  e = 1;
+  fn3c (g);
+  if (!*f)
+    abort ();
+  e = 1;
+  fn3d (g);
+  if (!*f)
+    abort ();
+  e = 1;
+  fn3e (g);
+  if (!*f)
+    abort ();
+  e = 1;
+  fn3f (g);
+  if (!*f)
+    abort ();
+  exit (0);
+}
+
diff --git a/gcc/toplev.c b/gcc/toplev.c
index feba051..76ce292 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1599,10 +1599,6 @@ backend_init_target (void)
   /* This depends on stack_pointer_rtx.  */
   init_fake_stack_mems ();
 
-  /* Sets static_base_value[HARD_FRAME_POINTER_REGNUM], which is
-     mode-dependent.  */
-  init_alias_target ();
-
   /* Depends on HARD_FRAME_POINTER_REGNUM.  */
   init_reload ();