i386: Use const reference of struct ix86_frame to avoid copy

Message ID 20180117160015.GA11885@gmail.com
State New
Headers show
Series
  • i386: Use const reference of struct ix86_frame to avoid copy
Related show

Commit Message

H.J. Lu Jan. 17, 2018, 4 p.m.
We can use const reference of struct ix86_frame to avoid making a local
copy of ix86_frame.  ix86_expand_epilogue makes a local copy of struct
ix86_frame and uses the reg_save_offset field as a local variable.  This
patch uses a separate local variable for reg_save_offset.

Tested on x86-64 with ada.  OK for trunk?

H.J.
--
	PR target/83905
	* config/i386/i386.c (ix86_expand_prologue): Use cost reference
	of struct ix86_frame.
	(ix86_expand_epilogue): Likewise.  Add a local variable for
	the reg_save_offset field in struct ix86_frame.
---
 gcc/config/i386/i386.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

-- 
2.14.3

Comments

Uros Bizjak Jan. 18, 2018, 8:23 a.m. | #1
On Wed, Jan 17, 2018 at 5:00 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> We can use const reference of struct ix86_frame to avoid making a local

> copy of ix86_frame.  ix86_expand_epilogue makes a local copy of struct

> ix86_frame and uses the reg_save_offset field as a local variable.  This

> patch uses a separate local variable for reg_save_offset.

>

> Tested on x86-64 with ada.  OK for trunk?


OK.

Thanks,
Uros.

> H.J.

> --

>         PR target/83905

>         * config/i386/i386.c (ix86_expand_prologue): Use cost reference

>         of struct ix86_frame.

>         (ix86_expand_epilogue): Likewise.  Add a local variable for

>         the reg_save_offset field in struct ix86_frame.

> ---

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

>  1 file changed, 12 insertions(+), 12 deletions(-)

>

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

> index a301e18ed70..340eca42449 100644

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

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

> @@ -13385,7 +13385,6 @@ ix86_expand_prologue (void)

>  {

>    struct machine_function *m = cfun->machine;

>    rtx insn, t;

> -  struct ix86_frame frame;

>    HOST_WIDE_INT allocate;

>    bool int_registers_saved;

>    bool sse_registers_saved;

> @@ -13413,7 +13412,7 @@ ix86_expand_prologue (void)

>    m->fs.sp_valid = true;

>    m->fs.sp_realigned = false;

>

> -  frame = m->frame;

> +  const struct ix86_frame &frame = cfun->machine->frame;

>

>    if (!TARGET_64BIT && ix86_function_ms_hook_prologue (current_function_decl))

>      {

> @@ -14291,7 +14290,6 @@ ix86_expand_epilogue (int style)

>  {

>    struct machine_function *m = cfun->machine;

>    struct machine_frame_state frame_state_save = m->fs;

> -  struct ix86_frame frame;

>    bool restore_regs_via_mov;

>    bool using_drap;

>    bool restore_stub_is_tail = false;

> @@ -14304,7 +14302,7 @@ ix86_expand_epilogue (int style)

>      }

>

>    ix86_finalize_stack_frame_flags ();

> -  frame = m->frame;

> +  const struct ix86_frame &frame = cfun->machine->frame;

>

>    m->fs.sp_realigned = stack_realign_fp;

>    m->fs.sp_valid = stack_realign_fp

> @@ -14348,11 +14346,13 @@ ix86_expand_epilogue (int style)

>                                   + UNITS_PER_WORD);

>      }

>

> +  HOST_WIDE_INT reg_save_offset = frame.reg_save_offset;

> +

>    /* Special care must be taken for the normal return case of a function

>       using eh_return: the eax and edx registers are marked as saved, but

>       not restored along this path.  Adjust the save location to match.  */

>    if (crtl->calls_eh_return && style != 2)

> -    frame.reg_save_offset -= 2 * UNITS_PER_WORD;

> +    reg_save_offset -= 2 * UNITS_PER_WORD;

>

>    /* EH_RETURN requires the use of moves to function properly.  */

>    if (crtl->calls_eh_return)

> @@ -14368,11 +14368,11 @@ ix86_expand_epilogue (int style)

>    else if (TARGET_EPILOGUE_USING_MOVE

>            && cfun->machine->use_fast_prologue_epilogue

>            && (frame.nregs > 1

> -              || m->fs.sp_offset != frame.reg_save_offset))

> +              || m->fs.sp_offset != reg_save_offset))

>      restore_regs_via_mov = true;

>    else if (frame_pointer_needed

>            && !frame.nregs

> -          && m->fs.sp_offset != frame.reg_save_offset)

> +          && m->fs.sp_offset != reg_save_offset)

>      restore_regs_via_mov = true;

>    else if (frame_pointer_needed

>            && TARGET_USE_LEAVE

> @@ -14440,7 +14440,7 @@ ix86_expand_epilogue (int style)

>        rtx t;

>

>        if (frame.nregs)

> -       ix86_emit_restore_regs_using_mov (frame.reg_save_offset, style == 2);

> +       ix86_emit_restore_regs_using_mov (reg_save_offset, style == 2);

>

>        /* eh_return epilogues need %ecx added to the stack pointer.  */

>        if (style == 2)

> @@ -14535,19 +14535,19 @@ ix86_expand_epilogue (int style)

>          in epilogues.  */

>        if (!m->fs.sp_valid || m->fs.sp_realigned

>           || (TARGET_SEH

> -             && (m->fs.sp_offset - frame.reg_save_offset

> +             && (m->fs.sp_offset - reg_save_offset

>                   >= SEH_MAX_FRAME_SIZE)))

>         {

>           pro_epilogue_adjust_stack (stack_pointer_rtx, hard_frame_pointer_rtx,

>                                      GEN_INT (m->fs.fp_offset

> -                                             - frame.reg_save_offset),

> +                                             - reg_save_offset),

>                                      style, false);

>         }

> -      else if (m->fs.sp_offset != frame.reg_save_offset)

> +      else if (m->fs.sp_offset != reg_save_offset)

>         {

>           pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,

>                                      GEN_INT (m->fs.sp_offset

> -                                             - frame.reg_save_offset),

> +                                             - reg_save_offset),

>                                      style,

>                                      m->fs.cfa_reg == stack_pointer_rtx);

>         }

> --

> 2.14.3

>

>
H.J. Lu Jan. 26, 2018, 11:42 p.m. | #2
On Thu, Jan 18, 2018 at 12:23 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Jan 17, 2018 at 5:00 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:

>> We can use const reference of struct ix86_frame to avoid making a local

>> copy of ix86_frame.  ix86_expand_epilogue makes a local copy of struct

>> ix86_frame and uses the reg_save_offset field as a local variable.  This

>> patch uses a separate local variable for reg_save_offset.

>>

>> Tested on x86-64 with ada.  OK for trunk?

>

> OK.

>


I'd like to backport it to gcc-7-branch.   Is that OK?

Thanks.


H.J.
>

>> H.J.

>> --

>>         PR target/83905

>>         * config/i386/i386.c (ix86_expand_prologue): Use cost reference

>>         of struct ix86_frame.

>>         (ix86_expand_epilogue): Likewise.  Add a local variable for

>>         the reg_save_offset field in struct ix86_frame.

>> ---

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

>>  1 file changed, 12 insertions(+), 12 deletions(-)

>>

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

>> index a301e18ed70..340eca42449 100644

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

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

>> @@ -13385,7 +13385,6 @@ ix86_expand_prologue (void)

>>  {

>>    struct machine_function *m = cfun->machine;

>>    rtx insn, t;

>> -  struct ix86_frame frame;

>>    HOST_WIDE_INT allocate;

>>    bool int_registers_saved;

>>    bool sse_registers_saved;

>> @@ -13413,7 +13412,7 @@ ix86_expand_prologue (void)

>>    m->fs.sp_valid = true;

>>    m->fs.sp_realigned = false;

>>

>> -  frame = m->frame;

>> +  const struct ix86_frame &frame = cfun->machine->frame;

>>

>>    if (!TARGET_64BIT && ix86_function_ms_hook_prologue (current_function_decl))

>>      {

>> @@ -14291,7 +14290,6 @@ ix86_expand_epilogue (int style)

>>  {

>>    struct machine_function *m = cfun->machine;

>>    struct machine_frame_state frame_state_save = m->fs;

>> -  struct ix86_frame frame;

>>    bool restore_regs_via_mov;

>>    bool using_drap;

>>    bool restore_stub_is_tail = false;

>> @@ -14304,7 +14302,7 @@ ix86_expand_epilogue (int style)

>>      }

>>

>>    ix86_finalize_stack_frame_flags ();

>> -  frame = m->frame;

>> +  const struct ix86_frame &frame = cfun->machine->frame;

>>

>>    m->fs.sp_realigned = stack_realign_fp;

>>    m->fs.sp_valid = stack_realign_fp

>> @@ -14348,11 +14346,13 @@ ix86_expand_epilogue (int style)

>>                                   + UNITS_PER_WORD);

>>      }

>>

>> +  HOST_WIDE_INT reg_save_offset = frame.reg_save_offset;

>> +

>>    /* Special care must be taken for the normal return case of a function

>>       using eh_return: the eax and edx registers are marked as saved, but

>>       not restored along this path.  Adjust the save location to match.  */

>>    if (crtl->calls_eh_return && style != 2)

>> -    frame.reg_save_offset -= 2 * UNITS_PER_WORD;

>> +    reg_save_offset -= 2 * UNITS_PER_WORD;

>>

>>    /* EH_RETURN requires the use of moves to function properly.  */

>>    if (crtl->calls_eh_return)

>> @@ -14368,11 +14368,11 @@ ix86_expand_epilogue (int style)

>>    else if (TARGET_EPILOGUE_USING_MOVE

>>            && cfun->machine->use_fast_prologue_epilogue

>>            && (frame.nregs > 1

>> -              || m->fs.sp_offset != frame.reg_save_offset))

>> +              || m->fs.sp_offset != reg_save_offset))

>>      restore_regs_via_mov = true;

>>    else if (frame_pointer_needed

>>            && !frame.nregs

>> -          && m->fs.sp_offset != frame.reg_save_offset)

>> +          && m->fs.sp_offset != reg_save_offset)

>>      restore_regs_via_mov = true;

>>    else if (frame_pointer_needed

>>            && TARGET_USE_LEAVE

>> @@ -14440,7 +14440,7 @@ ix86_expand_epilogue (int style)

>>        rtx t;

>>

>>        if (frame.nregs)

>> -       ix86_emit_restore_regs_using_mov (frame.reg_save_offset, style == 2);

>> +       ix86_emit_restore_regs_using_mov (reg_save_offset, style == 2);

>>

>>        /* eh_return epilogues need %ecx added to the stack pointer.  */

>>        if (style == 2)

>> @@ -14535,19 +14535,19 @@ ix86_expand_epilogue (int style)

>>          in epilogues.  */

>>        if (!m->fs.sp_valid || m->fs.sp_realigned

>>           || (TARGET_SEH

>> -             && (m->fs.sp_offset - frame.reg_save_offset

>> +             && (m->fs.sp_offset - reg_save_offset

>>                   >= SEH_MAX_FRAME_SIZE)))

>>         {

>>           pro_epilogue_adjust_stack (stack_pointer_rtx, hard_frame_pointer_rtx,

>>                                      GEN_INT (m->fs.fp_offset

>> -                                             - frame.reg_save_offset),

>> +                                             - reg_save_offset),

>>                                      style, false);

>>         }

>> -      else if (m->fs.sp_offset != frame.reg_save_offset)

>> +      else if (m->fs.sp_offset != reg_save_offset)

>>         {

>>           pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,

>>                                      GEN_INT (m->fs.sp_offset

>> -                                             - frame.reg_save_offset),

>> +                                             - reg_save_offset),

>>                                      style,

>>                                      m->fs.cfa_reg == stack_pointer_rtx);

>>         }

>> --

>> 2.14.3

>>

>>




-- 
H.J.
Uros Bizjak Jan. 27, 2018, 9:04 a.m. | #3
On Sat, Jan 27, 2018 at 12:42 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Jan 18, 2018 at 12:23 AM, Uros Bizjak <ubizjak@gmail.com> wrote:

>> On Wed, Jan 17, 2018 at 5:00 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:

>>> We can use const reference of struct ix86_frame to avoid making a local

>>> copy of ix86_frame.  ix86_expand_epilogue makes a local copy of struct

>>> ix86_frame and uses the reg_save_offset field as a local variable.  This

>>> patch uses a separate local variable for reg_save_offset.

>>>

>>> Tested on x86-64 with ada.  OK for trunk?

>>

>> OK.

>>

>

> I'd like to backport it to gcc-7-branch.   Is that OK?


OK.

Thanks,
Uros.

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index a301e18ed70..340eca42449 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13385,7 +13385,6 @@  ix86_expand_prologue (void)
 {
   struct machine_function *m = cfun->machine;
   rtx insn, t;
-  struct ix86_frame frame;
   HOST_WIDE_INT allocate;
   bool int_registers_saved;
   bool sse_registers_saved;
@@ -13413,7 +13412,7 @@  ix86_expand_prologue (void)
   m->fs.sp_valid = true;
   m->fs.sp_realigned = false;
 
-  frame = m->frame;
+  const struct ix86_frame &frame = cfun->machine->frame;
 
   if (!TARGET_64BIT && ix86_function_ms_hook_prologue (current_function_decl))
     {
@@ -14291,7 +14290,6 @@  ix86_expand_epilogue (int style)
 {
   struct machine_function *m = cfun->machine;
   struct machine_frame_state frame_state_save = m->fs;
-  struct ix86_frame frame;
   bool restore_regs_via_mov;
   bool using_drap;
   bool restore_stub_is_tail = false;
@@ -14304,7 +14302,7 @@  ix86_expand_epilogue (int style)
     }
 
   ix86_finalize_stack_frame_flags ();
-  frame = m->frame;
+  const struct ix86_frame &frame = cfun->machine->frame;
 
   m->fs.sp_realigned = stack_realign_fp;
   m->fs.sp_valid = stack_realign_fp
@@ -14348,11 +14346,13 @@  ix86_expand_epilogue (int style)
 				  + UNITS_PER_WORD);
     }
 
+  HOST_WIDE_INT reg_save_offset = frame.reg_save_offset;
+
   /* Special care must be taken for the normal return case of a function
      using eh_return: the eax and edx registers are marked as saved, but
      not restored along this path.  Adjust the save location to match.  */
   if (crtl->calls_eh_return && style != 2)
-    frame.reg_save_offset -= 2 * UNITS_PER_WORD;
+    reg_save_offset -= 2 * UNITS_PER_WORD;
 
   /* EH_RETURN requires the use of moves to function properly.  */
   if (crtl->calls_eh_return)
@@ -14368,11 +14368,11 @@  ix86_expand_epilogue (int style)
   else if (TARGET_EPILOGUE_USING_MOVE
 	   && cfun->machine->use_fast_prologue_epilogue
 	   && (frame.nregs > 1
-	       || m->fs.sp_offset != frame.reg_save_offset))
+	       || m->fs.sp_offset != reg_save_offset))
     restore_regs_via_mov = true;
   else if (frame_pointer_needed
 	   && !frame.nregs
-	   && m->fs.sp_offset != frame.reg_save_offset)
+	   && m->fs.sp_offset != reg_save_offset)
     restore_regs_via_mov = true;
   else if (frame_pointer_needed
 	   && TARGET_USE_LEAVE
@@ -14440,7 +14440,7 @@  ix86_expand_epilogue (int style)
       rtx t;
 
       if (frame.nregs)
-	ix86_emit_restore_regs_using_mov (frame.reg_save_offset, style == 2);
+	ix86_emit_restore_regs_using_mov (reg_save_offset, style == 2);
 
       /* eh_return epilogues need %ecx added to the stack pointer.  */
       if (style == 2)
@@ -14535,19 +14535,19 @@  ix86_expand_epilogue (int style)
 	 in epilogues.  */
       if (!m->fs.sp_valid || m->fs.sp_realigned
  	  || (TARGET_SEH
-	      && (m->fs.sp_offset - frame.reg_save_offset
+	      && (m->fs.sp_offset - reg_save_offset
 		  >= SEH_MAX_FRAME_SIZE)))
 	{
 	  pro_epilogue_adjust_stack (stack_pointer_rtx, hard_frame_pointer_rtx,
 				     GEN_INT (m->fs.fp_offset
-					      - frame.reg_save_offset),
+					      - reg_save_offset),
 				     style, false);
 	}
-      else if (m->fs.sp_offset != frame.reg_save_offset)
+      else if (m->fs.sp_offset != reg_save_offset)
 	{
 	  pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
 				     GEN_INT (m->fs.sp_offset
-					      - frame.reg_save_offset),
+					      - reg_save_offset),
 				     style,
 				     m->fs.cfa_reg == stack_pointer_rtx);
 	}