public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Daniel Santos <daniel.santos@pobox.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>,
	Uros Bizjak <ubizjak@gmail.com>,	Jan Hubicka <hubicka@ucw.cz>
Cc: Martin Liska <mliska@suse.cz>,	"H . J . Lu" <hjl.tools@gmail.com>
Subject: [PATCH 5/6 v2] [i386] Modify SP realignment in ix86_expand_prologue, et. al.
Date: Wed, 02 Aug 2017 23:28:00 -0000	[thread overview]
Message-ID: <20170802233344.24736-1-daniel.santos@pobox.com> (raw)
In-Reply-To: <20170731112435.30101-5-daniel.santos@pobox.com>

My first version of this patch inited m->fs.sp_realigned_fp_last with
the value of m->fs.sp_offset prior to performing the stack realignment.
I had forgotten, however, that when we're saving GP regs using MOV that
we delay SP modification as long as possible so that the value of
m->fs.sp_offset at this point is correct when we've used push, but
incorrect when we've used mov.

This time I've bootstraped with --enable-checking=yes,rtl
--enable-languages=all and reg tested using the below command to test both 64-
and 32-bit code.

  make -kj8 RUNTESTFLAGS="--target_board=unix/\{,-m32\}" check

Original patch description:

The SP allocation calculation is now done in ix86_compute_frame_layout
and the result stored in ix86_frame::stack_realign_allocate.  This
change also updates comments for choose_baseaddr to clarify that the
alignment returned doesn't necessarily reflect the alignment of the
cfa_offset passed (e.g., you can pass cfa_offset 48 and it can return an
alignment of 64 bytes).

Since the alignment required may be more than 16-bytes, we cannot defer
SP allocation to ix86_emit_outlined_ms2sysv_save (when it's enabled), so
that function needs to be updated as well.

Signed-off-by: Daniel Santos <daniel.santos@pobox.com>
---
 gcc/config/i386/i386.c | 58 ++++++++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 26 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 0dc366cf16e..a1f39cd714c 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13289,10 +13289,13 @@ choose_basereg (HOST_WIDE_INT cfa_offset, rtx &base_reg,
 }
 
 /* Return an RTX that points to CFA_OFFSET within the stack frame and
-   the alignment of address.  If align is non-null, it should point to
+   the alignment of address.  If ALIGN is non-null, it should point to
    an alignment value (in bits) that is preferred or zero and will
-   recieve the alignment of the base register that was selected.  The
-   valid base registers are taken from CFUN->MACHINE->FS.  */
+   recieve the alignment of the base register that was selected,
+   irrespective of rather or not CFA_OFFSET is a multiple of that
+   alignment value.
+
+   The valid base registers are taken from CFUN->MACHINE->FS.  */
 
 static rtx
 choose_baseaddr (HOST_WIDE_INT cfa_offset, unsigned int *align)
@@ -14338,35 +14341,35 @@ ix86_emit_outlined_ms2sysv_save (const struct ix86_frame &frame)
   rtx sym, addr;
   rtx rax = gen_rtx_REG (word_mode, AX_REG);
   const struct xlogue_layout &xlogue = xlogue_layout::get_instance ();
-  HOST_WIDE_INT rax_offset = xlogue.get_stub_ptr_offset () + m->fs.sp_offset;
-  HOST_WIDE_INT stack_alloc_size = frame.stack_pointer_offset - m->fs.sp_offset;
-  HOST_WIDE_INT stack_align_off_in = xlogue.get_stack_align_off_in ();
+  HOST_WIDE_INT allocate = frame.stack_pointer_offset - m->fs.sp_offset;
+
+  /* AL should only be live with sysv_abi.  */
+  gcc_assert (!ix86_eax_live_at_start_p ());
+
+  /* Setup RAX as the stub's base pointer.  We use stack_realign_offset rather
+     we've actually realigned the stack or not.  */
+  align = GET_MODE_ALIGNMENT (V4SFmode);
+  addr = choose_baseaddr (frame.stack_realign_offset
+			  + xlogue.get_stub_ptr_offset (), &align);
+  gcc_assert (align >= GET_MODE_ALIGNMENT (V4SFmode));
+  emit_insn (gen_rtx_SET (rax, addr));
 
-  /* Verify that the incoming stack 16-byte alignment offset matches the
-     layout we're using.  */
-  gcc_assert (stack_align_off_in == (m->fs.sp_offset & UNITS_PER_WORD));
+  /* Allocate stack if not already done.  */
+  if (allocate > 0)
+      pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
+				GEN_INT (-allocate), -1, false);
 
   /* Get the stub symbol.  */
   sym = xlogue.get_stub_rtx (frame_pointer_needed ? XLOGUE_STUB_SAVE_HFP
 						  : XLOGUE_STUB_SAVE);
   RTVEC_ELT (v, vi++) = gen_rtx_USE (VOIDmode, sym);
 
-  /* Setup RAX as the stub's base pointer.  */
-  align = GET_MODE_ALIGNMENT (V4SFmode);
-  addr = choose_baseaddr (rax_offset, &align);
-  gcc_assert (align >= GET_MODE_ALIGNMENT (V4SFmode));
-  insn = emit_insn (gen_rtx_SET (rax, addr));
-
-  gcc_assert (stack_alloc_size >= xlogue.get_stack_space_used ());
-  pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
-			     GEN_INT (-stack_alloc_size), -1,
-			     m->fs.cfa_reg == stack_pointer_rtx);
   for (i = 0; i < ncregs; ++i)
     {
       const xlogue_layout::reginfo &r = xlogue.get_reginfo (i);
       rtx reg = gen_rtx_REG ((SSE_REGNO_P (r.regno) ? V4SFmode : word_mode),
 			     r.regno);
-      RTVEC_ELT (v, vi++) = gen_frame_store (reg, rax, -r.offset);;
+      RTVEC_ELT (v, vi++) = gen_frame_store (reg, rax, -r.offset);
     }
 
   gcc_assert (vi == (unsigned)GET_NUM_ELEM (v));
@@ -14621,14 +14624,15 @@ ix86_expand_prologue (void)
       gcc_assert (align_bytes > MIN_STACK_BOUNDARY / BITS_PER_UNIT);
 
       /* Record last valid frame pointer offset.  */
-      m->fs.sp_realigned_fp_last = m->fs.sp_offset;
+      m->fs.sp_realigned_fp_last = frame.reg_save_offset;
 
       /* The computation of the size of the re-aligned stack frame means
 	 that we must allocate the size of the register save area before
 	 performing the actual alignment.  Otherwise we cannot guarantee
 	 that there's enough storage above the realignment point.  */
-      allocate = frame.stack_realign_allocate_offset - m->fs.sp_offset;
-      if (allocate && !m->call_ms2sysv)
+      allocate = frame.reg_save_offset - m->fs.sp_offset
+		 + frame.stack_realign_allocate;
+      if (allocate)
         pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
 				   GEN_INT (-allocate), -1, false);
 
@@ -14637,8 +14641,8 @@ ix86_expand_prologue (void)
 					stack_pointer_rtx,
 					GEN_INT (-align_bytes)));
       m->fs.sp_offset = ROUND_UP (m->fs.sp_offset, align_bytes);
-      m->fs.sp_realigned = true;
-      m->fs.sp_realigned_offset = m->fs.sp_offset - frame.nsseregs * 16;
+      m->fs.sp_realigned_offset = m->fs.sp_offset
+					      - frame.stack_realign_allocate;
       /* The stack pointer may no longer be equal to CFA - m->fs.sp_offset.
 	 Beyond this point, stack access should be done via choose_baseaddr or
 	 by using sp_valid_at and fp_valid_at to determine the correct base
@@ -14646,6 +14650,8 @@ ix86_expand_prologue (void)
 	 and not physical.  */
       gcc_assert (m->fs.sp_realigned_offset >= m->fs.sp_realigned_fp_last);
       gcc_assert (m->fs.sp_realigned_offset == frame.stack_realign_offset);
+      m->fs.sp_realigned = true;
+
       /* SEH unwind emit doesn't currently support REG_CFA_EXPRESSION, which
 	 is needed to describe where a register is saved using a realigned
 	 stack pointer, so we need to invalidate the stack pointer for that
@@ -14707,7 +14713,7 @@ ix86_expand_prologue (void)
      so probe if the size is non-negative to preserve the protection area.  */
   if (allocate >= 0 && flag_stack_check == STATIC_BUILTIN_STACK_CHECK)
     {
-      /* We expect the registers to be saved when probes are used.  */
+      /* We expect the GP registers to be saved when probes are used.  */
       gcc_assert (int_registers_saved);
 
       if (STACK_CHECK_MOVING_SP)
-- 
2.13.3

  reply	other threads:[~2017-08-02 23:28 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-31 11:16 [PATCH 0/6] [i386] PR80969 Fix ICE with -mabi=ms -mavx512f Daniel Santos
2017-07-31 11:19 ` [PATCH 4/6] [i386] Modify ix86_compute_frame_layout Daniel Santos
2017-07-31 11:19 ` [PATCH 2/6] [i386] Remove ix86_frame::outlined_save_offset Daniel Santos
2017-07-31 13:53   ` Uros Bizjak
2017-07-31 11:19 ` [PATCH 3/6] [i386] Remove machine_function::call_ms2sysv_pad_out Daniel Santos
2017-07-31 13:59   ` Uros Bizjak
2017-07-31 11:19 ` [PATCH 6/6] [i386, testsuite] Add tests, fix bug in check_avx2_hw_available Daniel Santos
2017-08-08 19:23   ` [PATCH 6/6 v2] " Daniel Santos
2017-07-31 11:19 ` [PATCH 5/6] [i386] Modify SP realignment in ix86_expand_prologue, et. al Daniel Santos
2017-08-02 23:28   ` Daniel Santos [this message]
2017-07-31 11:19 ` [PATCH 1/6] [i386] Correct comments, add assertions to sp_valid_at and fp_valid_at Daniel Santos
2017-07-31 17:23 ` [PATCH 0/6] [i386] PR80969 Fix ICE with -mabi=ms -mavx512f Daniel Santos
2017-08-01  6:20   ` Uros Bizjak
2017-08-08 19:31 ` PING " Daniel Santos
2017-08-22 22:44   ` [PATCH v4 0/4] " Daniel Santos
2017-08-22 23:23     ` [PATCH 1/4] [i386] Correct comments, add assertions to sp_valid_at and fp_valid_at Daniel Santos
2017-08-23  3:51     ` [PATCH 4/4] [i386, testsuite] Add tests, fix bug in check_avx2_hw_available Daniel Santos
2017-08-23 13:46       ` Uros Bizjak
2017-08-24  1:32         ` Daniel Santos
2017-08-23  4:17     ` [PATCH 2/4] [i386] Modify ix86_compute_frame_layout Daniel Santos
2017-08-23  4:18     ` [PATCH 3/4] [i386] Modify SP realignment in ix86_expand_prologue, et. al Daniel Santos
2017-08-23 13:53     ` [PATCH v4 0/4] [i386] PR80969 Fix ICE with -mabi=ms -mavx512f Uros Bizjak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170802233344.24736-1-daniel.santos@pobox.com \
    --to=daniel.santos@pobox.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hjl.tools@gmail.com \
    --cc=hubicka@ucw.cz \
    --cc=mliska@suse.cz \
    --cc=ubizjak@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).