i386: Align stack frame if argument is passed on stack

Message ID 20180110204011.GA21548@gmail.com
State New
Headers show
Series
  • i386: Align stack frame if argument is passed on stack
Related show

Commit Message

H.J. Lu Jan. 10, 2018, 8:40 p.m.
When a function call is removed, it may become a leaf function.  But if
argument may be passed on stack, we need to align the stack frame when
there is no tail call.

Tested on Linux/i686 and Linux/x86-64.

OK for trunk?

H.J.
---
gcc/

	PR target/83330
	* config/i386/i386.c (ix86_compute_frame_layout): Align stack
	frame if argument is passed on stack.

gcc/testsuite/

	PR target/83330
	* gcc.target/i386/pr83330.c: New test.
---
 gcc/config/i386/i386.c                  |  7 ++++++-
 gcc/testsuite/gcc.target/i386/pr83330.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr83330.c

-- 
2.14.3

Comments

Uros Bizjak Jan. 11, 2018, 7:07 p.m. | #1
On Wed, Jan 10, 2018 at 9:40 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> When a function call is removed, it may become a leaf function.  But if

> argument may be passed on stack, we need to align the stack frame when

> there is no tail call.

>

> Tested on Linux/i686 and Linux/x86-64.

>

> OK for trunk?

>

> H.J.

> ---

> gcc/

>

>         PR target/83330

>         * config/i386/i386.c (ix86_compute_frame_layout): Align stack

>         frame if argument is passed on stack.

>

> gcc/testsuite/

>

>         PR target/83330

>         * gcc.target/i386/pr83330.c: New test.


LGTM.

Thanks,
Uros.

> ---

>  gcc/config/i386/i386.c                  |  7 ++++++-

>  gcc/testsuite/gcc.target/i386/pr83330.c | 29 +++++++++++++++++++++++++++++

>  2 files changed, 35 insertions(+), 1 deletion(-)

>  create mode 100644 gcc/testsuite/gcc.target/i386/pr83330.c

>

> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c

> index 5e17b694d7f..d6ff096d466 100644

> --- a/gcc/config/i386/i386.c

> +++ b/gcc/config/i386/i386.c

> @@ -11339,11 +11339,16 @@ ix86_compute_frame_layout (void)

>        offset += frame->va_arg_size;

>      }

>

> -  /* Align start of frame for local function.  */

> +  /* Align start of frame for local function.  When a function call

> +     is removed, it may become a leaf function.  But if argument may

> +     be passed on stack, we need to align the stack when there is no

> +     tail call.  */

>    if (m->call_ms2sysv

>        || frame->va_arg_size != 0

>        || size != 0

>        || !crtl->is_leaf

> +      || (!crtl->tail_call_emit

> +         && cfun->machine->outgoing_args_on_stack)

>        || cfun->calls_alloca

>        || ix86_current_function_calls_tls_descriptor)

>      offset = ROUND_UP (offset, stack_alignment_needed);

> diff --git a/gcc/testsuite/gcc.target/i386/pr83330.c b/gcc/testsuite/gcc.target/i386/pr83330.c

> new file mode 100644

> index 00000000000..8a63fbd5d09

> --- /dev/null

> +++ b/gcc/testsuite/gcc.target/i386/pr83330.c

> @@ -0,0 +1,29 @@

> +/* { dg-do run { target int128 } } */

> +/* { dg-options "-O2 -fno-tree-dce -mno-push-args" } */

> +

> +typedef unsigned long long u64;

> +typedef unsigned __int128 u128;

> +

> +u64 v;

> +u64 g;

> +

> +u64 __attribute__ ((noinline, noclone))

> +bar (u128 d, u64 e, u64 f, u64 g, u128 h)

> +{

> +  (void)d, (void)e, (void)f, (void)g, (void)h;

> +  return 0;

> +}

> +

> +static u64 __attribute__ ((noipa))

> +foo (void)

> +{

> +  (void)(v - bar (0, 0, 0, 0, 0));

> +  return g;

> +}

> +

> +int

> +main (void)

> +{

> +  (void)foo ();

> +  return 0;

> +}

> --

> 2.14.3

>
H.J. Lu Jan. 11, 2018, 9:17 p.m. | #2
On Thu, Jan 11, 2018 at 11:07 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Jan 10, 2018 at 9:40 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:

>> When a function call is removed, it may become a leaf function.  But if

>> argument may be passed on stack, we need to align the stack frame when

>> there is no tail call.

>>

>> Tested on Linux/i686 and Linux/x86-64.

>>

>> OK for trunk?

>>

>> H.J.

>> ---

>> gcc/

>>

>>         PR target/83330

>>         * config/i386/i386.c (ix86_compute_frame_layout): Align stack

>>         frame if argument is passed on stack.

>>

>> gcc/testsuite/

>>

>>         PR target/83330

>>         * gcc.target/i386/pr83330.c: New test.

>

> LGTM.

>


Here is the backport for GCC 7.  OK for gcc-7-branch after a few days?

Thanks.

-- 
H.J.
From 28be904309cb8e6568e26f47a9746c709ac65fb8 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 6 Jul 2017 08:58:46 -0700
Subject: [PATCH] i386: Align stack frame if argument is passed on stack

When a function call is removed, it may become a leaf function.  But if
argument may be passed on stack, we need to align the stack frame when
there is no tail call.

gcc/

	Backport from mainline
	PR target/83330
	* config/i386/i386.c (ix86_function_arg_advance): Set
	outgoing_args_on_stack to true if there are outgoing arguments
	on stack.
	(ix86_function_arg): Likewise.
	(ix86_compute_frame_layout): Align stack frame if argument is
	passed on stack.
	* config/i386/i386.h (machine_function): Add
	outgoing_args_on_stack.

gcc/testsuite/

	Backport from mainline
	PR target/83330
	* gcc.target/i386/pr83330.c: New test.
---
 gcc/config/i386/i386.c                  | 19 +++++++++++++++++--
 gcc/config/i386/i386.h                  |  3 +++
 gcc/testsuite/gcc.target/i386/pr83330.c | 29 +++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr83330.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 8a3782c0298..4cc59e164ed 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -9799,7 +9799,13 @@ ix86_function_arg_advance (cumulative_args_t cum_v, machine_mode mode,
   /* For pointers passed in memory we expect bounds passed in Bounds
      Table.  */
   if (!nregs)
-    cum->bnds_in_bt = chkp_type_bounds_count (type);
+    {
+      /* Track if there are outgoing arguments on stack.  */
+      if (cum->caller)
+	cfun->machine->outgoing_args_on_stack = true;
+
+      cum->bnds_in_bt = chkp_type_bounds_count (type);
+    }
 }
 
 /* Define where to put the arguments to a function.
@@ -10128,6 +10134,10 @@ ix86_function_arg (cumulative_args_t cum_v, machine_mode omode,
   else
     arg = function_arg_32 (cum, mode, omode, type, bytes, words);
 
+  /* Track if there are outgoing arguments on stack.  */
+  if (arg == NULL_RTX && cum->caller)
+    cfun->machine->outgoing_args_on_stack = true;
+
   return arg;
 }
 
@@ -12557,11 +12567,16 @@ ix86_compute_frame_layout (struct ix86_frame *frame)
   frame->va_arg_size = ix86_varargs_gpr_size + ix86_varargs_fpr_size;
   offset += frame->va_arg_size;
 
-  /* Align start of frame for local function.  */
+  /* Align start of frame for local function.  When a function call
+     is removed, it may become a leaf function.  But if argument may
+     be passed on stack, we need to align the stack when there is no
+     tail call.  */
   if (stack_realign_fp
       || offset != frame->sse_reg_save_offset
       || size != 0
       || !crtl->is_leaf
+      || (!crtl->tail_call_emit
+	  && cfun->machine->outgoing_args_on_stack)
       || cfun->calls_alloca
       || ix86_current_function_calls_tls_descriptor)
     offset = ROUND_UP (offset, stack_alignment_needed);
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 9c776dc5172..7c651d7b944 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -2570,6 +2570,9 @@ struct GTY(()) machine_function {
      pass arguments and can be used for indirect sibcall.  */
   BOOL_BITFIELD arg_reg_available : 1;
 
+  /* Nonzero if the function places outgoing arguments on stack.  */
+  BOOL_BITFIELD outgoing_args_on_stack : 1;
+
   /* During prologue/epilogue generation, the current frame state.
      Otherwise, the frame state at the end of the prologue.  */
   struct machine_frame_state fs;
diff --git a/gcc/testsuite/gcc.target/i386/pr83330.c b/gcc/testsuite/gcc.target/i386/pr83330.c
new file mode 100644
index 00000000000..9040168377a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr83330.c
@@ -0,0 +1,29 @@
+/* { dg-do run { target int128 } } */
+/* { dg-options "-O2 -fno-tree-dce -mno-push-args" } */
+
+typedef unsigned long long u64;
+typedef unsigned __int128 u128;
+
+u64 v;
+u64 g;
+
+u64 __attribute__ ((noinline, noclone))
+bar (u128 d, u64 e, u64 f, u64 g, u128 h)
+{
+  (void)d, (void)e, (void)f, (void)g, (void)h;
+  return 0;
+}
+
+static u64
+foo (void)
+{
+  (void)(v - bar (0, 0, 0, 0, 0));
+  return g;
+}
+
+int
+main (void)
+{
+  (void)foo ();
+  return 0;
+}
Uros Bizjak Jan. 12, 2018, 4:57 p.m. | #3
On Thu, Jan 11, 2018 at 10:17 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Jan 11, 2018 at 11:07 AM, Uros Bizjak <ubizjak@gmail.com> wrote:

>> On Wed, Jan 10, 2018 at 9:40 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:

>>> When a function call is removed, it may become a leaf function.  But if

>>> argument may be passed on stack, we need to align the stack frame when

>>> there is no tail call.

>>>

>>> Tested on Linux/i686 and Linux/x86-64.

>>>

>>> OK for trunk?

>>>

>>> H.J.

>>> ---

>>> gcc/

>>>

>>>         PR target/83330

>>>         * config/i386/i386.c (ix86_compute_frame_layout): Align stack

>>>         frame if argument is passed on stack.

>>>

>>> gcc/testsuite/

>>>

>>>         PR target/83330

>>>         * gcc.target/i386/pr83330.c: New test.

>>

>> LGTM.

>>

>

> Here is the backport for GCC 7.  OK for gcc-7-branch after a few days?


OK.

Thanks,
Uros.

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 5e17b694d7f..d6ff096d466 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -11339,11 +11339,16 @@  ix86_compute_frame_layout (void)
       offset += frame->va_arg_size;
     }
 
-  /* Align start of frame for local function.  */
+  /* Align start of frame for local function.  When a function call
+     is removed, it may become a leaf function.  But if argument may
+     be passed on stack, we need to align the stack when there is no
+     tail call.  */
   if (m->call_ms2sysv
       || frame->va_arg_size != 0
       || size != 0
       || !crtl->is_leaf
+      || (!crtl->tail_call_emit
+	  && cfun->machine->outgoing_args_on_stack)
       || cfun->calls_alloca
       || ix86_current_function_calls_tls_descriptor)
     offset = ROUND_UP (offset, stack_alignment_needed);
diff --git a/gcc/testsuite/gcc.target/i386/pr83330.c b/gcc/testsuite/gcc.target/i386/pr83330.c
new file mode 100644
index 00000000000..8a63fbd5d09
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr83330.c
@@ -0,0 +1,29 @@ 
+/* { dg-do run { target int128 } } */
+/* { dg-options "-O2 -fno-tree-dce -mno-push-args" } */
+
+typedef unsigned long long u64;
+typedef unsigned __int128 u128;
+
+u64 v;
+u64 g;
+
+u64 __attribute__ ((noinline, noclone))
+bar (u128 d, u64 e, u64 f, u64 g, u128 h)
+{
+  (void)d, (void)e, (void)f, (void)g, (void)h;
+  return 0;
+}
+
+static u64 __attribute__ ((noipa))
+foo (void)
+{
+  (void)(v - bar (0, 0, 0, 0, 0));
+  return g;
+}
+
+int
+main (void)
+{
+  (void)foo ();
+  return 0;
+}