On Thu, 2023-12-07 at 09:40 +0800, Yang Yujie wrote:
>  static void
>  loongarch_for_each_saved_reg (HOST_WIDE_INT sp_offset,
> -                           loongarch_save_restore_fn fn)
> +                           loongarch_save_restore_fn fn,
> +                           bool skip_eh_data_regs_p)
>  {
>    HOST_WIDE_INT offset;
>  
>    /* Save the link register and s-registers.  */
>    offset = cfun->machine->frame.gp_sp_offset - sp_offset;
>    for (int regno = GP_REG_FIRST; regno <= GP_REG_LAST; regno++)
> -    if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
> -      {
> -     if (!cfun->machine->reg_is_wrapped_separately[regno])
> -       loongarch_save_restore_reg (word_mode, regno, offset, fn);
> +    {
> +      /* Special care needs to be taken for $r4-$r7 (EH_RETURN_DATA_REGNO)
> +      when returning normally from a function that calls __builtin_eh_return.
> +      In this case, these registers are saved but should not be restored,
> +      or the return value may be clobbered.  */
>  
> -     offset -= UNITS_PER_WORD;
> -      }
> +      if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
> +     {
> +       if (!(cfun->machine->reg_is_wrapped_separately[regno]
> +             || (skip_eh_data_regs_p
> +             && GP_ARG_FIRST <= regno && regno < GP_ARG_FIRST + 4)))
> +         loongarch_save_restore_reg (word_mode, regno, offset, fn);
> +
> +       offset -= UNITS_PER_WORD;
> +     }
> +    }

I don't like this pair of {} for the for statement.  It's not necessary
and it changes the indent level, causing the diff hard to review.

Otherwise LGTM.  I'm not sure why I didn't notice the eh_return issue
when I learnt shrink wrapping from RISC-V...

-- 
Xi Ruoyao <xry...@xry111.site>
School of Aerospace Science and Technology, Xidian University

Reply via email to