diff mbox

[v2,i386] : Fix PR69140, stack alignment + O1 breaks with Microsoft ABI

Message ID CAFULd4ZVetz0YFYWAnbxda=HbuQHjVyB6yYDGGOiri+XeW_wJQ@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak Jan. 7, 2016, 7:11 p.m. UTC 
On Wed, Jan 6, 2016 at 9:16 PM, Uros Bizjak <ubizjak@gmail.com> wrote:

> Attached patch fixes a failure, where no base register can be found to
> save the remaining SSE (and integer) registers to stack. Since we are
> able to restore registers in the prollogue using:
>
>   m->fs.sp_valid = (!frame_pointer_needed
>    || (crtl->sp_is_unchanging
> && !stack_realign_fp));
>
> we can use similar approach at the end of the ix86_expand_prologue:
>
>   m->fs.sp_valid = !frame_pointer_needed;
>
> This way, we will always use frame pointer, if one is available or if
> frame pointer is not available, the (realigned) stack register for
> saves.
>
> 2016-01-06  Uros Bizjak  <ubizjak@gmail.com>
>
>     PR target/69140
>     * config/i386/i386.c (ix86_expand_prologue): Declare fs.sp_valid
>     depending on frame_pointer_needed before remaining integer and SSE
>     registers are saved.
>
> testsuite/ChangeLog:
>
> 2016-01-06  Uros Bizjak  <ubizjak@gmail.com>
>
>     PR target/69140
>     * gcc.target/i386/pr69140.c: New test
>
> Patch was bootstrapped and regression tested on x86_64-linux-gnu for
> all default languages, Obj-C++ and Go. The patch was also tested by
> the reporter by building and running Wine without problems.
>
> I have to admit that prologue generation is hard to test, since it has
> many different code paths. I guess committing the one-liner in the
> hope that it won't break some target is the best approach one can do.
>
> So, the patch was committed to mainline. I plan to backport it to
> gcc-5 branch after a week or two without problems in mainline.


Ouch, we can't just make %rsp valid, it doesn't point to CFA_OFFSET
when realigned. I will revert previous patch and enable frame-pointer
for realigned MS_ABI functions instead (if frame-pointer generation
is not needed, then it will be disabled in
ix86_finalize_stack_realign_flags anyway).

2016-01-07  Uros Bizjak  <ubizjak@gmail.com>

    PR target/69140
    * config/i386/i386.c (ix86_frame_pointer_required): Enable
    frame pointer for TARGET_64BIT_MS_ABI when stack is misaligned.

2016-01-07  Uros Bizjak  <ubizjak@gmail.com>

    Revert
    2016-01-06  Uros Bizjak  <ubizjak@gmail.com>

    PR target/69140
    * config/i386/i386.c (ix86_expand_prologue): Declare fs.sp_valid
    depending on frame_pointer_needed before remaining integer and SSE
    registers are saved.

Tested on x86_64-linux-gnu {,-m32}  and committed to mainline SVN.

Uros.
diff mbox

Patch

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 232131)
+++ ChangeLog	(working copy)
@@ -1,3 +1,19 @@ 
+2016-01-07  Uros Bizjak  <ubizjak@gmail.com>
+
+	PR target/69140
+	* config/i386/i386.c (ix86_frame_pointer_required): Enable
+	frame pointer for TARGET_64BIT_MS_ABI when stack is misaligned.
+
+2016-01-07  Uros Bizjak  <ubizjak@gmail.com>
+
+	Revert
+	2016-01-06  Uros Bizjak  <ubizjak@gmail.com>
+
+	PR target/69140
+	* config/i386/i386.c (ix86_expand_prologue): Declare fs.sp_valid
+	depending on frame_pointer_needed before remaining integer and SSE
+	registers are saved.
+
 2016-01-07  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR target/69171
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 232131)
+++ config/i386/i386.c	(working copy)
@@ -10903,6 +10903,10 @@  ix86_frame_pointer_required (void)
   if (TARGET_64BIT_MS_ABI && get_frame_size () > SEH_MAX_FRAME_SIZE)
     return true;
 
+  /* SSE saves require frame-pointer when stack is misaligned.  */
+  if (TARGET_64BIT_MS_ABI && ix86_incoming_stack_boundary < 128)
+    return true;
+  
   /* In ix86_option_override_internal, TARGET_OMIT_LEAF_FRAME_POINTER
      turns off the frame pointer by default.  Turn it back on now if
      we've not got a leaf function.  */
@@ -13065,8 +13069,6 @@  ix86_expand_prologue (void)
       m->fs.fp_valid = true;
     }
 
-  m->fs.sp_valid = !frame_pointer_needed;
-
   if (!int_registers_saved)
     ix86_emit_save_regs_using_mov (frame.reg_save_offset);
   if (!sse_registers_saved)