[PATCH] Fix up my recent PR59501 i?86 changes (PR target/59644)

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

As discussed in the PR, my recent patch broke the Linux kernel.
The problem is that if register allocation calls ix86_compute_frame_layout
to determine elimination offsets and the DRAP register is assumed saved at
that point, but later on during pro_epilogue pass
ix86_finalize_stack_realign_flags determines we don't need to save/restore
the DRAP register, it is in some cases already too late, as the size of the
DRAP register save slot is accounted for in the RA elimination offsets in
instructions (both for fp -> hfp elimination or even argp -> sp).

So, this patch essentially reverts part of those changes, the only way to do
it if there are rsp/rbp references in the function is to do this during LRA
(perhaps some target hook), if there is some point where we know we won't
need further vector mode spill slots and thus can know whether we'll need
dynamic stack readjustment or not, and could still change elimination
offsets.  For the easy case where a leaf function doesn't mention rsp/rbp
anywhere we can still avoid saving/restoring the DRAP register unnecessarily
as the patch does, if it is needed at all, and if not continue to not set it
up at all nor save/restore.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2014-01-06  Jakub Jelinek  <jakub@redhat.com>

	PR target/59644
	* config/i386/i386.h (struct machine_function): Add
	no_drap_save_restore field.
	* config/i386/i386.c (ix86_save_reg): Use
	!cfun->machine->no_drap_save_restore instead of
	crtl->stack_realign_needed.
	(ix86_finalize_stack_realign_flags): Don't clear drap_reg unless
	this function clears frame_pointer_needed.  Set
	cfun->machine->no_drap_save_restore if clearing frame_pointer_needed
	and DRAP reg is needed.

	* gcc.target/i386/pr59644.c: New test.

--- gcc/config/i386/i386.h.jj	2014-01-03 11:41:06.000000000 +0100
+++ gcc/config/i386/i386.h	2014-01-06 13:41:10.445337916 +0100
@@ -2419,6 +2419,9 @@ struct GTY(()) machine_function {
      stack below the return address.  */
   BOOL_BITFIELD static_chain_on_stack : 1;
 
+  /* If true, it is safe to not save/restore DRAP register.  */
+  BOOL_BITFIELD no_drap_save_restore : 1;
+
   /* During prologue/epilogue generation, the current frame state.
      Otherwise, the frame state at the end of the prologue.  */
   struct machine_frame_state fs;
--- gcc/config/i386/i386.c.jj	2014-01-04 10:56:54.000000000 +0100
+++ gcc/config/i386/i386.c	2014-01-06 13:42:53.016821514 +0100
@@ -9281,7 +9281,7 @@ ix86_save_reg (unsigned int regno, bool
 
   if (crtl->drap_reg
       && regno == REGNO (crtl->drap_reg)
-      && crtl->stack_realign_needed)
+      && !cfun->machine->no_drap_save_restore)
     return true;
 
   return (df_regs_ever_live_p (regno)
@@ -10519,18 +10519,6 @@ ix86_finalize_stack_realign_flags (void)
       return;
     }
 
-  /* If drap has been set, but it actually isn't live at the start
-     of the function and !stack_realign, there is no reason to set it up.  */
-  if (crtl->drap_reg && !stack_realign)
-    {
-      basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
-      if (! REGNO_REG_SET_P (DF_LR_IN (bb), REGNO (crtl->drap_reg)))
-	{
-	  crtl->drap_reg = NULL_RTX;
-	  crtl->need_drap = false;
-	}
-    }
-
   /* If the only reason for frame_pointer_needed is that we conservatively
      assumed stack realignment might be needed, but in the end nothing that
      needed the stack alignment had been spilled, clear frame_pointer_needed
@@ -10584,6 +10572,8 @@ ix86_finalize_stack_realign_flags (void)
 	      crtl->need_drap = false;
 	    }
 	}
+      else
+	cfun->machine->no_drap_save_restore = true;
 
       frame_pointer_needed = false;
       stack_realign = false;
--- gcc/testsuite/gcc.target/i386/pr59644.c.jj	2014-01-06 14:19:52.201616003 +0100
+++ gcc/testsuite/gcc.target/i386/pr59644.c	2014-01-06 14:22:04.606940097 +0100
@@ -0,0 +1,42 @@
+/* PR target/59644 */
+/* { dg-do run { target lp64 } } */
+/* { dg-options "-O2 -ffreestanding -mno-sse -mpreferred-stack-boundary=3 -maccumulate-outgoing-args -mno-red-zone" } */
+
+/* This test uses __builtin_trap () instead of e.g. abort,
+   because due to -mpreferred-stack-boundary=3 it should not call
+   any library function from within main ().  */
+
+#include <stdarg.h>
+
+__attribute__((noinline, noclone))
+int
+bar (int x, int y, int z, int w, const char *fmt, va_list ap)
+{
+  if (x != 1 || y != 2 || z != 3 || w != 4)
+    __builtin_trap ();
+  if (fmt[0] != 'f' || fmt[1] != 'o' || fmt[2] != 'o' || fmt[3])
+    __builtin_trap ();
+  if (va_arg (ap, int) != 5 || va_arg (ap, int) != 6
+      || va_arg (ap, long long) != 7LL)
+    __builtin_trap ();
+  return 9;
+}
+
+__attribute__((noinline, noclone, optimize ("Os")))
+int
+foo (const char *fmt, ...)
+{
+  va_list ap;
+  va_start (ap, fmt);
+  int r = bar (1, 2, 3, 4, fmt, ap);
+  va_end (ap);
+  return r;
+}
+
+int
+main ()
+{
+  if (foo ("foo", 5, 6, 7LL) != 9)
+    __builtin_trap ();
+  return 0;
+}

	Jakub