Hi Jakub,

Thanks for doing this!

在 2024/11/7 20:16, Jakub Jelinek 写道:
> Hi!
> 
> The following patch on top of the
> https://gcc.gnu.org/pipermail/gcc-patches/2024-November/667949.html
> patch adds rs6000 part of the support (the only other target I'm aware of
> which clearly has red zone as well).
> 
> 2024-11-07  Jakub Jelinek  <ja...@redhat.com>
> 
>       * config/rs6000/rs6000.h (struct machine_function): Add
>       asm_redzone_clobber_seen member.
>       * config/rs6000/rs6000-logue.cc (rs6000_stack_info): Force
>       info->push_p if cfun->machine->asm_redzone_clobber_seen.
>       * config/rs6000/rs6000.cc (TARGET_REDZONE_CLOBBER): Redefine.
>       (rs6000_redzone_clobber): New function.
> 
>       * gcc.target/powerpc/asm-redzone-1.c: New test.
> 
> --- gcc/config/rs6000/rs6000.h.jj     2024-08-30 09:09:45.407624634 +0200
> +++ gcc/config/rs6000/rs6000.h        2024-11-07 12:25:44.979466003 +0100
> @@ -2424,6 +2424,7 @@ typedef struct GTY(()) machine_function
>       global entry.  It helps to control the patchable area before and after
>       local entry.  */
>    bool global_entry_emitted;
> +  bool asm_redzone_clobber_seen;
>  } machine_function;
>  #endif
>  
> --- gcc/config/rs6000/rs6000-logue.cc.jj      2024-10-25 10:00:29.389768987 
> +0200
> +++ gcc/config/rs6000/rs6000-logue.cc 2024-11-07 12:36:05.985688899 +0100
> @@ -918,7 +918,7 @@ rs6000_stack_info (void)
>    else if (DEFAULT_ABI == ABI_V4)
>      info->push_p = non_fixed_size != 0;
>  
> -  else if (frame_pointer_needed)
> +  else if (frame_pointer_needed || cfun->machine->asm_redzone_clobber_seen)
>      info->push_p = 1;
>  
>    else
> --- gcc/config/rs6000/rs6000.cc.jj    2024-10-25 10:00:29.393768930 +0200
> +++ gcc/config/rs6000/rs6000.cc       2024-11-07 12:34:21.679163134 +0100
> @@ -1752,6 +1752,9 @@ static const scoped_attribute_specs *con
>  #undef TARGET_CAN_CHANGE_MODE_CLASS
>  #define TARGET_CAN_CHANGE_MODE_CLASS rs6000_can_change_mode_class
>  
> +#undef TARGET_REDZONE_CLOBBER
> +#define TARGET_REDZONE_CLOBBER rs6000_redzone_clobber
> +
>  #undef TARGET_CONSTANT_ALIGNMENT
>  #define TARGET_CONSTANT_ALIGNMENT rs6000_constant_alignment
>  
> @@ -13727,6 +13730,24 @@ rs6000_can_change_mode_class (machine_mo
>    return true;
>  }
>  
> +/* Implement TARGET_REDZONE_CLOBBER.  */
> +
> +static rtx
> +rs6000_redzone_clobber ()
> +{
> +  cfun->machine->asm_redzone_clobber_seen = true;
> +  if (DEFAULT_ABI != ABI_V4)
> +    {
> +      int red_zone_size = TARGET_32BIT ? 220 : 288;

I think this is perfectly consistent with what we have in
function offset_below_red_zone_p.  But I noticed that elfv2
ABI has document mentioning red zone of 512 bytes:

2.2.2.4 Protected Zone

The 288 bytes below the stack pointer are available as volatile program storage 
that is not preserved across
function calls. Interrupt handlers and any other functions that might run 
without an explicit call must take care
to preserve a protected zone, also referred to as the red zone, of 512 bytes 
that consists of:
• The 288-byte volatile program storage region that is used to hold saved 
registers and local variables
• An additional 224 bytes below the volatile program storage region that is set 
aside as a volatile system
storage region for system functions
...

It looks like an oversight instead of intentional setting in
the current offset_below_red_zone_p, looking forward to Segher's
comments. :)

BR,
Kewen

> +      rtx base = plus_constant (Pmode, stack_pointer_rtx,
> +                             GEN_INT (-red_zone_size));
> +      rtx mem = gen_rtx_MEM (BLKmode, base);
> +      set_mem_size (mem, red_zone_size);
> +      return mem;
> +    }
> +  return NULL_RTX;
> +}
> +
>  /* Debug version of rs6000_can_change_mode_class.  */
>  static bool
>  rs6000_debug_can_change_mode_class (machine_mode from,
> --- gcc/testsuite/gcc.target/powerpc/asm-redzone-1.c.jj       2024-11-07 
> 13:01:36.935064863 +0100
> +++ gcc/testsuite/gcc.target/powerpc/asm-redzone-1.c  2024-11-07 
> 13:01:31.449142367 +0100
> @@ -0,0 +1,71 @@
> +/* { dg-do run { target lp64 } } */
> +/* { dg-options "-O2" } */
> +
> +__attribute__((noipa)) int
> +foo (void)
> +{
> +  int a = 1;
> +  int b = 2;
> +  int c = 3;
> +  int d = 4;
> +  int e = 5;
> +  int f = 6;
> +  int g = 7;
> +  int h = 8;
> +  int i = 9;
> +  int j = 10;
> +  int k = 11;
> +  int l = 12;
> +  int m = 13;
> +  int n = 14;
> +  int o = 15;
> +  int p = 16;
> +  int q = 17;
> +  int r = 18;
> +  int s = 19;
> +  int t = 20;
> +  int u = 21;
> +  int v = 22;
> +  int w = 23;
> +  int x = 24;
> +  int y = 25;
> +  int z = 26;
> +  asm volatile ("" : "+g" (a), "+g" (b), "+g" (c), "+g" (d), "+g" (e));
> +  asm volatile ("" : "+g" (f), "+g" (g), "+g" (h), "+g" (i), "+g" (j));
> +  asm volatile ("" : "+g" (k), "+g" (l), "+g" (m), "+g" (n), "+g" (o));
> +  asm volatile ("" : "+g" (k), "+g" (l), "+g" (m), "+g" (n), "+g" (o));
> +  asm volatile ("" : "+g" (p), "+g" (q), "+g" (s), "+g" (t), "+g" (u));
> +  asm volatile ("" : "+g" (v), "+g" (w), "+g" (y), "+g" (z));
> +#ifdef __PPC64__
> +  asm volatile ("std 1,-8(1); std 1,-16(1); std 1,-24(1); std 1,-32(1)"
> +             : : : "18", "19", "20", "redzone");
> +#elif defined(_AIX)
> +  asm volatile ("stw 1,-4(1); stw 1,-8(1); stw 1,-12(1); stw 1,-16(1)"
> +             : : : "18", "19", "20", "redzone");
> +#endif
> +  asm volatile ("" : "+g" (a), "+g" (b), "+g" (c), "+g" (d), "+g" (e));
> +  asm volatile ("" : "+g" (f), "+g" (g), "+g" (h), "+g" (i), "+g" (j));
> +  asm volatile ("" : "+g" (k), "+g" (l), "+g" (m), "+g" (n), "+g" (o));
> +  asm volatile ("" : "+g" (p), "+g" (q), "+g" (s), "+g" (t), "+g" (u));
> +  asm volatile ("" : "+g" (v), "+g" (w), "+g" (y), "+g" (z));
> +  return a + b + c + d + e + f + g + h + i + j + k + l + m + n;
> +}
> +
> +__attribute__((noipa)) void
> +bar (char *p, long *q)
> +{
> +  (void) p;
> +  *q = 42;
> +}
> +
> +int
> +main ()
> +{
> +  volatile int x = 256;
> +  long y;
> +  bar (__builtin_alloca (x), &y);
> +  if (foo () != 105)
> +    __builtin_abort ();
> +  if (y != 42)
> +    __builtin_abort ();
> +}
> 
>       Jakub

Reply via email to