RE: [PATCH] Fix stack red zone bug (PR38644)

["Jiangning Liu" <jiangning dot liu at arm dot com>]

Just realized ChangeLog needs to be changed as well.

ChangeLog:

        * config/i386/i386.c (ix86_using_red_zone): Remove inline.
        (TARGET_STACK_USING_RED_ZONE): New.
        * config/rs6000/rs6000.c (rs6000_stack_using_red_zone): New.
        (TARGET_STACK_USING_RED_ZONE): New.
        (offset_below_red_zone_p): Change to use new hook 
        TARGET_STACK_USING_RED_ZONE.
        * doc/tm.texi (TARGET_STACK_USING_RED_ZONE): New.
        * doc/tm.texi.in (TARGET_STACK_USING_RED_ZONE): New.
        * sched-deps.c (sched_analyze_1): If the stack pointer is being 
        modified and stack red zone is not supported for ports, flush out 
        all memory references as they may become invalid if moved across 
        the stack adjustment.
        * target.def (stack_using_red_zone): New.
        * testsuite/gcc.target/arm/stack-red-zone.c: New.

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Jiangning Liu
> Sent: Wednesday, September 28, 2011 2:24 PM
> To: 'Uros Bizjak'
> Cc: gcc-patches@gcc.gnu.org; jh@suse.cz; geoffk@geoffk.org;
> dje.gcc@gmail.com; rth@redhat.com; Richard Earnshaw; Matthew Gretton-
> Dann
> Subject: RE: [PATCH] Fix stack red zone bug (PR38644)
> 
> > > -static inline bool
> > > +extern bool
> >
> > static bool
> >
> > > ?ix86_using_red_zone (void)
> > > ?{
> > > ? return TARGET_RED_ZONE && !TARGET_64BIT_MS_ABI;
> > > @@ -35958,6 +35958,9 @@ ix86_autovectorize_vector_sizes (void)
> > > ?#define TARGET_STACK_PROTECT_FAIL ix86_stack_protect_fail
> > > ?#endif
> > >
> 
> ...
> 
> > >
> > > +/* Return true if the ABI allows red zone access. ?*/
> > > +extern bool
> >
> > static bool
> >
> > > +rs6000_stack_using_red_zone (void)
> > > +{
> > > + ? return (DEFAULT_ABI != ABI_V4);
> > > +}
> > > +
> >
> 
> Uros,
> 
> Thanks for your review. Accept and new patch is as below. No change for
> ChangeLog.
> 
> Thanks,
> -Jiangning
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index ff8c49f..e200974 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -2631,7 +2631,7 @@ static const char *const
> cpu_names[TARGET_CPU_DEFAULT_max] =
> 
> 
>  /* Return true if a red-zone is in use.  */
> 
> -static inline bool
> +static bool
>  ix86_using_red_zone (void)
>  {
>    return TARGET_RED_ZONE && !TARGET_64BIT_MS_ABI;
> @@ -35958,6 +35958,9 @@ ix86_autovectorize_vector_sizes (void)
>  #define TARGET_STACK_PROTECT_FAIL ix86_stack_protect_fail
>  #endif
> 
> +#undef TARGET_STACK_USING_RED_ZONE
> +#define TARGET_STACK_USING_RED_ZONE ix86_using_red_zone
> +
>  #undef TARGET_FUNCTION_VALUE
>  #define TARGET_FUNCTION_VALUE ix86_function_value
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 1ab57e5..1e64d14 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -1537,6 +1537,9 @@ static const struct attribute_spec
> rs6000_attribute_table[] =
>  #define TARGET_STACK_PROTECT_FAIL rs6000_stack_protect_fail
>  #endif
> 
> +#undef TARGET_STACK_USING_RED_ZONE
> +#define TARGET_STACK_USING_RED_ZONE rs6000_stack_using_red_zone
> +
>  /* MPC604EUM 3.5.2 Weak Consistency between Multiple Processors
>     The PowerPC architecture requires only weak consistency among
>     processors--that is, memory accesses between processors need not be
> @@ -20660,6 +20663,13 @@ rs6000_restore_saved_cr (rtx reg, int
> using_mfcr_multiple)
>  	}
>  }
> 
> +/* Return true if the ABI allows red zone access.  */
> +static bool
> +rs6000_stack_using_red_zone (void)
> +{
> +   return (DEFAULT_ABI != ABI_V4);
> +}
> +
>  /* Return true if OFFSET from stack pointer can be clobbered by
> signals.
>     V.4 doesn't have any stack cushion, AIX ABIs have 220 or 288 bytes
>     below stack pointer not cloberred by signals.  */
> @@ -20667,7 +20677,7 @@ rs6000_restore_saved_cr (rtx reg, int
> using_mfcr_multiple)
>  static inline bool
>  offset_below_red_zone_p (HOST_WIDE_INT offset)
>  {
> -  return offset < (DEFAULT_ABI == ABI_V4
> +  return offset < (!TARGET_STACK_USING_RED_ZONE
>  		   ? 0
>  		   : TARGET_32BIT ? -220 : -288);
>  }
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 335c1d1..c848806 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -11332,6 +11332,22 @@ to the stack.  Therefore, this hook should
> return
> true in general, but
>  false for naked functions.  The default implementation always returns
> true.
>  @end deftypefn
> 
> +@deftypefn {Target Hook} bool TARGET_STACK_USING_RED_ZONE (void)
> +This hook returns true if the target has a red zone (an area beyond
> the
> +current extent of the stack that cannot be modified by asynchronous
> events
> +on the processor).
> +
> +If this hook returns false then the compiler mid-end will not move an
> access
> +to memory in the stack frame past a stack adjustment insn.
> +
> +If this hook returns true then the compiler mid-end will assume that
> it is
> +safe to move an access to memory in the stack frame past a stack
> adjustment
> +insn.  The target back-end must emit scheduling barrier insns when
> this is
> +unsafe.
> +
> +The default is to return false which is safe and appropriate for most
> targets.
> +@end deftypefn
> +
>  @deftypevr {Target Hook} {unsigned HOST_WIDE_INT} TARGET_CONST_ANCHOR
>  On some architectures it can take multiple instructions to synthesize
>  a constant.  If there is another constant already in a register that
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index 6783826..0fa9e10 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -11223,6 +11223,22 @@ to the stack.  Therefore, this hook should
> return
> true in general, but
>  false for naked functions.  The default implementation always returns
> true.
>  @end deftypefn
> 
> +@hook TARGET_STACK_USING_RED_ZONE
> +This hook returns true if the target has a red zone (an area beyond
> the
> +current extent of the stack that cannot be modified by asynchronous
> events
> +on the processor).
> +
> +If this hook returns false then the compiler mid-end will not move an
> access
> +to memory in the stack frame past a stack adjustment insn.
> +
> +If this hook returns true then the compiler mid-end will assume that
> it is
> +safe to move an access to memory in the stack frame past a stack
> adjustment
> +insn.  The target back-end must emit scheduling barrier insns when
> this is
> +unsafe.
> +
> +The default is to return false which is safe and appropriate for most
> targets.
> +@end deftypefn
> +
>  @hook TARGET_CONST_ANCHOR
>  On some architectures it can take multiple instructions to synthesize
>  a constant.  If there is another constant already in a register that
> diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
> index ed592c8..575b20c 100644
> --- a/gcc/sched-deps.c
> +++ b/gcc/sched-deps.c
> @@ -2332,6 +2332,14 @@ sched_analyze_1 (struct deps_desc *deps, rtx x,
> rtx
> insn)
>  			       FIRST_STACK_REG);
>  	}
>  #endif
> +      /* If the stack pointer is being modified, flush out all memory
> +         references as they may become invalid if moved across the
> +         stack adjustment.  */
> +      if (!targetm.calls.stack_using_red_zone ()
> +          && (dest == stack_pointer_rtx))
> +        {
> +          flush_pending_lists (deps, insn, true, true);
> +        }
>      }
>    else if (MEM_P (dest))
>      {
> diff --git a/gcc/target.def b/gcc/target.def
> index 1e09ba7..5e2cd38 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -2110,6 +2110,13 @@ DEFHOOK
>   bool, (void),
>   hook_bool_void_true)
> 
> +/* Return true if target uses stack red zone.  */
> +DEFHOOK
> +(stack_using_red_zone,
> + "",
> + bool, (void),
> + hook_bool_void_false)
> +
>  /* Return an rtx for the static chain for FNDECL.  If INCOMING_P is
> true,
>         then it should be for the callee; otherwise for the caller.  */
>  DEFHOOK
> diff --git a/gcc/testsuite/gcc.target/arm/stack-red-zone.c
> b/gcc/testsuite/gcc.target/arm/stack-red-zone.c
> new file mode 100644
> index 0000000..4888707
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/stack-red-zone.c
> @@ -0,0 +1,13 @@
> +/* No stack red zone.  PR38644.  */
> +/* { dg-options "-mthumb -O2" } */
> +/* { dg-final { scan-assembler "ldrb\[^\n\]*\\n\[\t \]*add\[\t
> \]*sp\[^\n\]*\\n\[\t \]*@\[^\n\]*\\n\[\t \]*pop" } } */
> +/* { dg-require-effective-target arm_thumb1_ok } */
> +
> +extern int doStreamReadBlock (int *, char *, int size, int);
> +
> +char readStream (int *s)
> +{
> +       char c = 0;
> +       doStreamReadBlock (s, &c, 1, *s);
> +       return c;
> +}
> 
> 
>