Re: PATCH [mainline] use of save_world/rest_world calls in apple-ppc-darwin prologue/epilogue

[Geoffrey Keating <geoffk at apple dot com>]

On 30/07/2004, at 5:02 PM, Fariborz Jahanian wrote:

> This patch introduces calls to save_world/rest_world in a few 
> darwin-abi prologue/epilogue's
> to avoid instruction-by-instruction store and load of all ppc/altivec 
> non-volatile GRR/FP/VRs
> as well as special registers. save_world/rest_world will do  all the 
> saves and restores thus
> saving code size. This mechanism has been in use for a few years in 
> apple's version of gcc's.
> We would like to include this in FSF mainline to avoid future merges. 
> darwin-world.asm file
> where save_world/rest_world is defined has been in FSF mainline for 
> some time now.

Are you sure this actually reduces system code size with GCC 3.5 (more 
than having this patch in GCC increases it)?  For instance, does it get 
used (more than once) in GCC itself?

> + (define_insn "*save_world"
> +  [(match_parallel 0 "any_operand"
> +                   [(clobber (match_operand:SI 1 "register_operand" 
> "=l"))
> +                    (use (match_operand:SI 2 "call_operand" "s"))
> +                    (set (match_operand:V4SI 3 "any_operand" "=m")
> +                         (match_operand:V4SI 4 "register_operand" 
> "v"))])]
> +  "TARGET_MACHO && (DEFAULT_ABI == ABI_DARWIN) && TARGET_32BIT"
> +  {
> +     return "bl %z2";
> +  }
> +   [(set_attr "type" "branch")
> +    (set_attr "length" "4")])

You need to use something more specific than 'any_operand', or some 
pass may decide to rewrite one of these patterns to something you 
didn't expect.

> !   if (TARGET_ALTIVEC_VRSAVE && info_ptr->vrsave_mask)
> !     info_ptr->vrsave_size  = 4;
>     else
> !     info_ptr->vrsave_size  = 0;
>
>     /* Calculate the offsets.  */
>     switch (DEFAULT_ABI)
> --- 11483,11497 ----
>     else
>       info_ptr->vrsave_mask = 0;
>
> !   if ((TARGET_ALTIVEC_VRSAVE && info_ptr->vrsave_mask)
> !   /* Even if we're not touching VRsave, make sure there's room on the
> !      stack for it, if it looks like we're calling SAVE_WORLD, which
> !      will attempt to save it. */
> !       || ((DEFAULT_ABI == ABI_DARWIN)
> ! 	  && (info_ptr->first_altivec_reg_save == 
> FIRST_SAVED_ALTIVEC_REGNO)))
> !       info_ptr->vrsave_size  = 4;
>     else
> !       info_ptr->vrsave_size  = 0;

This test should use 'info_ptr->world_save_p' rather than guessing.  
(You would have to move this code down after world_save_p is set.)

> +       /* The goto below skips over this, so replicate here.  */
> +       if (current_function_calls_eh_return)
> +         {
> +           unsigned int i;
> +           for (i = 0; ; ++i)
> +             {
> +               unsigned int regno = EH_RETURN_DATA_REGNO (i);
> +               if (regno == INVALID_REGNUM)
> +                 break;
> +               emit_frame_save (frame_reg_rtx, frame_ptr_rtx, 
> reg_mode, regno,
> +                                info->ehrd_offset + sp_offset
> +                                + reg_size * (int) i,
> +                                info->total_size);
> +             }
> +         }
> +       goto world_saved;
> +     }
> +
>     /* Save AltiVec registers if needed.  */
>     if (TARGET_ALTIVEC_ABI && info->altivec_size != 0)
>       {
> *************** rs6000_emit_prologue (void)
> *** 12912,12917 ****
> --- 13081,13087 ----
>         && !(DEFAULT_ABI == ABI_V4 || 
> current_function_calls_eh_return))
>       rs6000_emit_allocate_stack (info->total_size, FALSE);
>
> + world_saved:
>     /* Set frame pointer, if needed.  */
>     if (frame_pointer_needed)
>       {

It would be better to explicitly test for ! info_ptr->world_save_p in 
each of the chunks of code, rather than have a goto, especially since 
the goto causes code duplication.

> + #define FIRST_SAVED_ALTIVEC_REGNO 97
> + #define FIRST_SAVED_FP_REGNO    (14+32)
> + #define FIRST_SAVED_GP_REGNO 13
> +

This should use FIRST_ALTIVEC_REGNO instead of the magic constant 97.

Attachment: smime.p7s
Description: S/MIME cryptographic signature