Re: [PATCH] i386: Use const reference of struct ix86_frame to avoid copy

["H.J. Lu" <hjl dot tools at gmail dot com>]

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.