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

[Jiangning Liu]

 -Original Message-
 From: Richard Henderson [mailto:r...@redhat.com]
 Sent: Saturday, October 01, 2011 3:05 AM
 To: Jiangning Liu
 Cc: 'Jakub Jelinek'; 'Richard Guenther'; Andrew Pinski; gcc-
 patc...@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
 On 09/29/2011 06:13 PM, Jiangning Liu wrote:
 
 
  -Original Message-
  From: Jakub Jelinek [mailto:ja...@redhat.com]
  Sent: Thursday, September 29, 2011 6:14 PM
  To: Jiangning Liu
  Cc: 'Richard Guenther'; Andrew Pinski; gcc-patches@gcc.gnu.org
  Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
  On Thu, Sep 29, 2011 at 06:08:50PM +0800, Jiangning Liu wrote:
  As far as I know different back-ends are implementing different
  prologue/epilogue in GCC. If one day this part can be refined and
  abstracted
  as well, I would say solving this stack-red-zone problem in shared
  prologue/epilogue code would be a perfect solution, and barrier can
  be
  inserted there.
 
  I'm not saying you are wrong on keeping scheduler using a pure
  barrier
  interface. From engineering point of view, I only feel my proposal
 is
  so far
  so good, because this patch at least solve the problem for all
  targets in a
  quite simple way. Maybe it can be improved in future based on this.
 
  But you don't want to listen about any other alternative, other
  backends are
  happy with being able to put the best kind of barrier at the best
 spot
  in the epilogue and don't need a generic solution which won't
 model
  very
  well the target diversity anyway.
 
  Jakub,
 
  Appreciate for your attention on this issue,
 
  1) Can you clarify who are the others back-ends? Does it cover most
 of the
  back-ends being supported by GCC right now?
 
 Your red-stack barrier issue is *exactly* the same as the frame pointer
 barrier issue, which affects many backends.
 
 That is, if the frame pointer is initialized before the local stack
 frame
 is allocated, then one has to add a barrier such that memory references
 based on the frame pointer are not scheduled before the local stack
 frame
 allocation.
 
 One example of this is in the i386 port, where the prologue looks like
 
   push%ebp
   mov %esp, %ebp
   sub $frame, %esp
 
 The rtl we emit for that subtraction looks like
 
 (define_insn pro_epilogue_adjust_stack_mode_add
   [(set (match_operand:P 0 register_operand =r,r)
 (plus:P (match_operand:P 1 register_operand 0,r)
 (match_operand:P 2 nonmemory_operand ri,li)))
(clobber (reg:CC FLAGS_REG))
(clobber (mem:BLK (scratch)))]
 
 Note the final clobber, which is a memory scheduling barrier.
 
 Other targets use similar tricks.  For instance arm stack_tie.
 
 Honestly, I've found nothing convincing throughout this thread that
 suggests to me that this problem should be handled generically.
 

Richard H.,

Thanks for your explanation by giving an example in x86. 

The key is if possible, fixing it in middle end can benefit all ports
directly and avoid bug fixing burden in back-ends, rather than fix this
problem port by port.

Actually now the debating here is whether memory barrier is properly
modeling through whole GCC rather than a single component, because my
current understanding is scheduler is not the only component using memory
barrier.

Thanks,
-Jiangning

 
 r~

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

[Jiangning Liu]

 -Original Message-
 From: Richard Guenther [mailto:richard.guent...@gmail.com]
 Sent: Friday, September 30, 2011 8:57 PM
 To: Jiangning Liu; Jakub Jelinek; Richard Guenther; Andrew Pinski; gcc-
 patc...@gcc.gnu.org; richard.sandif...@linaro.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
 On Fri, Sep 30, 2011 at 2:46 PM, Richard Sandiford
 richard.sandif...@linaro.org wrote:
  Jiangning Liu jiangning@arm.com writes:
  You seem to feel strongly about this because it's a wrong-code bug
 that
  is very easy to introduce and often very hard to detect.  And I
  defintely
  sympathise with that.  If we were going to to do it in a target-
  independent
  way, though, I think it would be better to scan patterns like
 epilogue
  and
  automatically introduce barriers before assignments to
  stack_pointer_rtx
  (subject to the kind of hook in your patch).  But I still don't
 think
  that's better than leaving the onus on the backend.  The backend is
  still responsible for much more complicated things like determning
  the correct deallocation and register-restore sequence, and for
  determining the correct CFI sequence.
 
 
  I think middle-end in GCC is actually shared code rather than the
 part
  exactly in the middle. A pass working on RTL can be a middle end
 just
  because the code can be shared for all targets, and some passes can
 even
  work for both GIMPLE and RTL.
 
  Actually some optimizations need to work through shared part
 (middle-end)
  plus target specific part (back-end). You are thinking the
 interface
  between this shared part and target specific part should be
 using
  barrier as a properly model. To some extension I agree with this.
 However,
  it doesn't mean the fix should be in back-end rather than middle end,
  because obviously this problem is a common ABI issue for all targets.
 If we
  can abstract this issue to be a shared part, why shouldn't we do it
 in
  middle end to reduce the onus of back-end? Back-end should handle
 the target
  specific things rather than only the complicated things.
 
  And for avoidance of doubt, the automatic barrier insertion that I
  described would be one way of doing it in target-independent code.
  But...
 
  If a complicated problem can be implemented in a shared code
 manner, we
  still want to put it into middle end rather than back-end. I believe
 those
  optimizations based on SSA form are complicated enough, but they are
 all in
  middle end. This is the logic I'm seeing in GCC.
 
  The situation here is different.  The target-independent rtl code is
  being given a blob of instructions that the backend has generated for
  the epilogue.  There's no fine-tuning beyond that.  E.g. we don't
 have
  separate patterns for restore registers, deallocate stack,
 return:
  we just have one monolithic epilogue pattern.  The target-
 independent
  code has very little control.
 
  In contrast, after the tree optimisers have handed off the initial IL,
  the tree optimisers are more or less in full control.  There are very
  few cases where we generate further trees outside the middle-
 end.  The only
  case I know off-hand is the innards of va_start and va_arg, which can
 be
  generated by the backend.
 
  So let's suppose we had a similar situation there, where we wanted
  va_arg do something special in a certain situation.  If we had the
  same three choices of:
 
   1. use an on-the-side hook to represent the special something
   2. scan the code generated by the backend and automatically
      inject the special something at an appropriate place
   3. require each backend to do it properly from the start
 
  (OK, slightly prejudiced wording :-)) I think we'd still choose 3.
 
  For this particular issue, I don't think that hook interface I'm
  proposing is more complicated than the barrier. Instead, it is
 easier
  for back-end implementer to be aware of the potential issue before
  really solving stack red zone problem, because it is very clearly
  listed in target hook list.
 
  The point for model it in the IL supporters like myself is that we
  have both many backends and many rtl passes.  Putting it in a hook
 keeps
  things simple for the backends, but it means that every rtl pass must
 be
  aware of this on-the-side dependency.  Perhaps sched2 really is the
 only
  pass that needs to look at the hook at present.  But perhaps not.
  E.g. dbr_schedule (not a problem on ARM, I realise) also reorders
  instructions, so maybe it would need to be audited to see whether any
  calls to this hook are needed.  And perhaps we'd add more rtl passes
  later.
 
  The point behind using a barrier is that the rtl passes do not then
 need
  to treat the stack-deallocation dependency as a special case.  They
 can
  just use the normal analysis and get it right.
 
  In other words, we're both arguing for safety here.
 
 Indeed.  It's certainly not only scheduling that can move instructions,
 but RTL PRE, combine, ifcvt all can

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

[Jiangning Liu]

 -Original Message-
 From: Richard Sandiford richard.sandif...@linaro.org
 Date: Fri, Sep 30, 2011 at 8:46 PM
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 To: Jiangning Liu jiangning@arm.com
 Cc: Jakub Jelinek ja...@redhat.com, Richard Guenther
 richard.guent...@gmail.com, Andrew Pinski pins...@gmail.com,
 gcc-patches@gcc.gnu.org
 
 
 Jiangning Liu jiangning@arm.com writes:
  You seem to feel strongly about this because it's a wrong-code bug
 that
  is very easy to introduce and often very hard to detect.  And I
  defintely
  sympathise with that.  If we were going to to do it in a target-
  independent
  way, though, I think it would be better to scan patterns like
 epilogue
  and
  automatically introduce barriers before assignments to
  stack_pointer_rtx
  (subject to the kind of hook in your patch).  But I still don't
 think
  that's better than leaving the onus on the backend.  The backend is
  still responsible for much more complicated things like determning
  the correct deallocation and register-restore sequence, and for
  determining the correct CFI sequence.
 
 
  I think middle-end in GCC is actually shared code rather than the
 part
  exactly in the middle. A pass working on RTL can be a middle end just
  because the code can be shared for all targets, and some passes can
 even
  work for both GIMPLE and RTL.
 
  Actually some optimizations need to work through shared part
 (middle-end)
  plus target specific part (back-end). You are thinking the
 interface
  between this shared part and target specific part should be using
  barrier as a properly model. To some extension I agree with this.
 However,
  it doesn't mean the fix should be in back-end rather than middle end,
  because obviously this problem is a common ABI issue for all targets.
 If we
  can abstract this issue to be a shared part, why shouldn't we do it
 in
  middle end to reduce the onus of back-end? Back-end should handle the
 target
  specific things rather than only the complicated things.
 
 And for avoidance of doubt, the automatic barrier insertion that I
 described would be one way of doing it in target-independent code.
 But...
 
  If a complicated problem can be implemented in a shared code manner,
 we
  still want to put it into middle end rather than back-end. I believe
 those
  optimizations based on SSA form are complicated enough, but they are
 all in
  middle end. This is the logic I'm seeing in GCC.
 
 The situation here is different.  The target-independent rtl code is
 being given a blob of instructions that the backend has generated for
 the epilogue.  There's no fine-tuning beyond that.  E.g. we don't have
 separate patterns for restore registers, deallocate stack, return:
 we just have one monolithic epilogue pattern.  The target-independent
 code has very little control.
 
 In contrast, after the tree optimisers have handed off the initial IL,
 the tree optimisers are more or less in full control.  There are very
 few cases where we generate further trees outside the middle-end.  The
 only
 case I know off-hand is the innards of va_start and va_arg, which can
 be
 generated by the backend.
 
 So let's suppose we had a similar situation there, where we wanted
 va_arg do something special in a certain situation.  If we had the
 same three choices of:
 
  1. use an on-the-side hook to represent the special something
  2. scan the code generated by the backend and automatically
     inject the special something at an appropriate place
  3. require each backend to do it properly from the start
 
 (OK, slightly prejudiced wording :-)) I think we'd still choose 3.
 

Richard S.,

Although I've ever implemented va_arg for a commercial compiler previously
long times ago, I forgot all the details. :-) I'm not sure if using va_arg
is a good example to compare with this stack red zone case.

  For this particular issue, I don't think that hook interface I'm
  proposing is more complicated than the barrier. Instead, it is easier
  for back-end implementer to be aware of the potential issue before
  really solving stack red zone problem, because it is very clearly
  listed in target hook list.
 
 The point for model it in the IL supporters like myself is that we
 have both many backends and many rtl passes.  Putting it in a hook
 keeps
 things simple for the backends, but it means that every rtl pass must
 be
 aware of this on-the-side dependency.  Perhaps sched2 really is the
 only
 pass that needs to look at the hook at present.  But perhaps not.
 E.g. dbr_schedule (not a problem on ARM, I realise) also reorders
 instructions, so maybe it would need to be audited to see whether any
 calls to this hook are needed.  And perhaps we'd add more rtl passes
 later.

Let me rephrase your justification with my own words.

===

We can't compare adding a new pass and adding a new port, because they are
totally different things. But it implies with my proposal the burden may
still be added

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

[Steven Bosscher]

On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu jiangning@arm.com wrote:
 4) There are over 300 TARGET HOOKS being defined in target.def. I don't
 think adding this interface is hurting GCC.

The reason why there are so many, is because everyone thinks what you
state here ;-)

Ciao!
Steven

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

[Richard Sandiford]

Jiangning Liu jiangning@arm.com writes:
 -Original Message-
 From: Jakub Jelinek [mailto:ja...@redhat.com]
 Sent: Thursday, September 29, 2011 6:14 PM
 To: Jiangning Liu
 Cc: 'Richard Guenther'; Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
 On Thu, Sep 29, 2011 at 06:08:50PM +0800, Jiangning Liu wrote:
  As far as I know different back-ends are implementing different
  prologue/epilogue in GCC. If one day this part can be refined and
 abstracted
  as well, I would say solving this stack-red-zone problem in shared
  prologue/epilogue code would be a perfect solution, and barrier can
 be
  inserted there.
 
  I'm not saying you are wrong on keeping scheduler using a pure
 barrier
  interface. From engineering point of view, I only feel my proposal is
 so far
  so good, because this patch at least solve the problem for all
 targets in a
  quite simple way. Maybe it can be improved in future based on this.
 
 But you don't want to listen about any other alternative, other
 backends are
 happy with being able to put the best kind of barrier at the best spot
 in the epilogue and don't need a generic solution which won't model
 very
 well the target diversity anyway.

 Jakub,

 Appreciate for your attention on this issue,

 1) Can you clarify who are the others back-ends? Does it cover most of the
 back-ends being supported by GCC right now?

Not answering for Jakub of course, but as a maintainer of a backend, I know
MIPS doesn't have the required barrier at the moment.  But that's a bug.

Like others in this thread, I'm strongly of the opinion that this should
be modelled directly in the IL.  And it's already supposed to be modelled
in the IL.  Target-independent code emits the required barriers in cases
where it rather than the backend patterns are responsible.  E.g.:

  emit_clobber (gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode)));
  emit_clobber (gen_rtx_MEM (BLKmode, hard_frame_pointer_rtx));

  emit_move_insn (hard_frame_pointer_rtx, fp);
  emit_stack_restore (SAVE_NONLOCAL, stack);

from expand_builtin_longjmp() and:

  if (sa != 0)
{
  sa = validize_mem (sa);
  /* These clobbers prevent the scheduler from moving
 references to variable arrays below the code
 that deletes (pops) the arrays.  */
  emit_clobber (gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode)));
  emit_clobber (gen_rtx_MEM (BLKmode, stack_pointer_rtx));
}

from emit_stack_restore().  Backends that fail to follow suit are IMO
just buggy.

FWIW, I intend to fix MIPS this weekend.

You seem to feel strongly about this because it's a wrong-code bug that
is very easy to introduce and often very hard to detect.  And I defintely
sympathise with that.  If we were going to to do it in a target-independent
way, though, I think it would be better to scan patterns like epilogue and
automatically introduce barriers before assignments to stack_pointer_rtx
(subject to the kind of hook in your patch).  But I still don't think
that's better than leaving the onus on the backend.  The backend is
still responsible for much more complicated things like determning
the correct deallocation and register-restore sequence, and for
determining the correct CFI sequence.

Richard

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

[Jiangning Liu]

 -Original Message-
 From: Richard Sandiford [mailto:rdsandif...@googlemail.com]
 Sent: Friday, September 30, 2011 4:15 PM
 To: Jiangning Liu
 Cc: 'Jakub Jelinek'; 'Richard Guenther'; Andrew Pinski; gcc-
 patc...@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
 Jiangning Liu jiangning@arm.com writes:
  -Original Message-
  From: Jakub Jelinek [mailto:ja...@redhat.com]
  Sent: Thursday, September 29, 2011 6:14 PM
  To: Jiangning Liu
  Cc: 'Richard Guenther'; Andrew Pinski; gcc-patches@gcc.gnu.org
  Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
  On Thu, Sep 29, 2011 at 06:08:50PM +0800, Jiangning Liu wrote:
   As far as I know different back-ends are implementing different
   prologue/epilogue in GCC. If one day this part can be refined and
  abstracted
   as well, I would say solving this stack-red-zone problem in shared
   prologue/epilogue code would be a perfect solution, and barrier
 can
  be
   inserted there.
  
   I'm not saying you are wrong on keeping scheduler using a pure
  barrier
   interface. From engineering point of view, I only feel my proposal
 is
  so far
   so good, because this patch at least solve the problem for all
  targets in a
   quite simple way. Maybe it can be improved in future based on this.
 
  But you don't want to listen about any other alternative, other
  backends are
  happy with being able to put the best kind of barrier at the best
 spot
  in the epilogue and don't need a generic solution which won't
 model
  very
  well the target diversity anyway.
 
  Jakub,
 
  Appreciate for your attention on this issue,
 
  1) Can you clarify who are the others back-ends? Does it cover most
 of the
  back-ends being supported by GCC right now?
 
 Not answering for Jakub of course, but as a maintainer of a backend, I
 know
 MIPS doesn't have the required barrier at the moment.  But that's a bug.
 
 Like others in this thread, I'm strongly of the opinion that this
 should
 be modelled directly in the IL.  And it's already supposed to be
 modelled
 in the IL.  Target-independent code emits the required barriers in
 cases
 where it rather than the backend patterns are responsible.  E.g.:
 
 emit_clobber (gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode)));
 emit_clobber (gen_rtx_MEM (BLKmode, hard_frame_pointer_rtx));
 
 emit_move_insn (hard_frame_pointer_rtx, fp);
 emit_stack_restore (SAVE_NONLOCAL, stack);
 
 from expand_builtin_longjmp() and:
 
   if (sa != 0)
 {
   sa = validize_mem (sa);
   /* These clobbers prevent the scheduler from moving
references to variable arrays below the code
that deletes (pops) the arrays.  */
   emit_clobber (gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode)));
   emit_clobber (gen_rtx_MEM (BLKmode, stack_pointer_rtx));
 }
 
 from emit_stack_restore().  Backends that fail to follow suit are IMO
 just buggy.
 
 FWIW, I intend to fix MIPS this weekend.

Richard S.,

Appreciate your attention on this issue and investigation on MIPS target.

Really glad to know you find a potential bug for MIPS through this
discussion. To some extension this proved my hypothesis previously.

 
 You seem to feel strongly about this because it's a wrong-code bug that
 is very easy to introduce and often very hard to detect.  And I
 defintely
 sympathise with that.  If we were going to to do it in a target-
 independent
 way, though, I think it would be better to scan patterns like epilogue
 and
 automatically introduce barriers before assignments to
 stack_pointer_rtx
 (subject to the kind of hook in your patch).  But I still don't think
 that's better than leaving the onus on the backend.  The backend is
 still responsible for much more complicated things like determning
 the correct deallocation and register-restore sequence, and for
 determining the correct CFI sequence.
 

I think middle-end in GCC is actually shared code rather than the part
exactly in the middle. A pass working on RTL can be a middle end just
because the code can be shared for all targets, and some passes can even
work for both GIMPLE and RTL.

Actually some optimizations need to work through shared part (middle-end)
plus target specific part (back-end). You are thinking the interface
between this shared part and target specific part should be using
barrier as a properly model. To some extension I agree with this. However,
it doesn't mean the fix should be in back-end rather than middle end,
because obviously this problem is a common ABI issue for all targets. If we
can abstract this issue to be a shared part, why shouldn't we do it in
middle end to reduce the onus of back-end? Back-end should handle the target
specific things rather than only the complicated things. 

If a complicated problem can be implemented in a shared code manner, we
still want to put it into middle end rather than back-end. I believe those
optimizations based on SSA form are complicated enough

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

[Richard Sandiford]

Jiangning Liu jiangning@arm.com writes:
 You seem to feel strongly about this because it's a wrong-code bug that
 is very easy to introduce and often very hard to detect.  And I
 defintely
 sympathise with that.  If we were going to to do it in a target-
 independent
 way, though, I think it would be better to scan patterns like epilogue
 and
 automatically introduce barriers before assignments to
 stack_pointer_rtx
 (subject to the kind of hook in your patch).  But I still don't think
 that's better than leaving the onus on the backend.  The backend is
 still responsible for much more complicated things like determning
 the correct deallocation and register-restore sequence, and for
 determining the correct CFI sequence.
 

 I think middle-end in GCC is actually shared code rather than the part
 exactly in the middle. A pass working on RTL can be a middle end just
 because the code can be shared for all targets, and some passes can even
 work for both GIMPLE and RTL.

 Actually some optimizations need to work through shared part (middle-end)
 plus target specific part (back-end). You are thinking the interface
 between this shared part and target specific part should be using
 barrier as a properly model. To some extension I agree with this. However,
 it doesn't mean the fix should be in back-end rather than middle end,
 because obviously this problem is a common ABI issue for all targets. If we
 can abstract this issue to be a shared part, why shouldn't we do it in
 middle end to reduce the onus of back-end? Back-end should handle the target
 specific things rather than only the complicated things. 

And for avoidance of doubt, the automatic barrier insertion that I
described would be one way of doing it in target-independent code.
But...

 If a complicated problem can be implemented in a shared code manner, we
 still want to put it into middle end rather than back-end. I believe those
 optimizations based on SSA form are complicated enough, but they are all in
 middle end. This is the logic I'm seeing in GCC.

The situation here is different.  The target-independent rtl code is
being given a blob of instructions that the backend has generated for
the epilogue.  There's no fine-tuning beyond that.  E.g. we don't have
separate patterns for restore registers, deallocate stack, return:
we just have one monolithic epilogue pattern.  The target-independent
code has very little control.

In contrast, after the tree optimisers have handed off the initial IL,
the tree optimisers are more or less in full control.  There are very
few cases where we generate further trees outside the middle-end.  The only
case I know off-hand is the innards of va_start and va_arg, which can be
generated by the backend.

So let's suppose we had a similar situation there, where we wanted
va_arg do something special in a certain situation.  If we had the
same three choices of:

  1. use an on-the-side hook to represent the special something
  2. scan the code generated by the backend and automatically
 inject the special something at an appropriate place
  3. require each backend to do it properly from the start

(OK, slightly prejudiced wording :-)) I think we'd still choose 3.

 For this particular issue, I don't think that hook interface I'm
 proposing is more complicated than the barrier. Instead, it is easier
 for back-end implementer to be aware of the potential issue before
 really solving stack red zone problem, because it is very clearly
 listed in target hook list.

The point for model it in the IL supporters like myself is that we
have both many backends and many rtl passes.  Putting it in a hook keeps
things simple for the backends, but it means that every rtl pass must be
aware of this on-the-side dependency.  Perhaps sched2 really is the only
pass that needs to look at the hook at present.  But perhaps not.
E.g. dbr_schedule (not a problem on ARM, I realise) also reorders
instructions, so maybe it would need to be audited to see whether any
calls to this hook are needed.  And perhaps we'd add more rtl passes
later.

The point behind using a barrier is that the rtl passes do not then need
to treat the stack-deallocation dependency as a special case.  They can
just use the normal analysis and get it right.

In other words, we're both arguing for safety here.

Richard

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

[Richard Guenther]

On Fri, Sep 30, 2011 at 2:46 PM, Richard Sandiford
richard.sandif...@linaro.org wrote:
 Jiangning Liu jiangning@arm.com writes:
 You seem to feel strongly about this because it's a wrong-code bug that
 is very easy to introduce and often very hard to detect.  And I
 defintely
 sympathise with that.  If we were going to to do it in a target-
 independent
 way, though, I think it would be better to scan patterns like epilogue
 and
 automatically introduce barriers before assignments to
 stack_pointer_rtx
 (subject to the kind of hook in your patch).  But I still don't think
 that's better than leaving the onus on the backend.  The backend is
 still responsible for much more complicated things like determning
 the correct deallocation and register-restore sequence, and for
 determining the correct CFI sequence.


 I think middle-end in GCC is actually shared code rather than the part
 exactly in the middle. A pass working on RTL can be a middle end just
 because the code can be shared for all targets, and some passes can even
 work for both GIMPLE and RTL.

 Actually some optimizations need to work through shared part (middle-end)
 plus target specific part (back-end). You are thinking the interface
 between this shared part and target specific part should be using
 barrier as a properly model. To some extension I agree with this. However,
 it doesn't mean the fix should be in back-end rather than middle end,
 because obviously this problem is a common ABI issue for all targets. If we
 can abstract this issue to be a shared part, why shouldn't we do it in
 middle end to reduce the onus of back-end? Back-end should handle the target
 specific things rather than only the complicated things.

 And for avoidance of doubt, the automatic barrier insertion that I
 described would be one way of doing it in target-independent code.
 But...

 If a complicated problem can be implemented in a shared code manner, we
 still want to put it into middle end rather than back-end. I believe those
 optimizations based on SSA form are complicated enough, but they are all in
 middle end. This is the logic I'm seeing in GCC.

 The situation here is different.  The target-independent rtl code is
 being given a blob of instructions that the backend has generated for
 the epilogue.  There's no fine-tuning beyond that.  E.g. we don't have
 separate patterns for restore registers, deallocate stack, return:
 we just have one monolithic epilogue pattern.  The target-independent
 code has very little control.

 In contrast, after the tree optimisers have handed off the initial IL,
 the tree optimisers are more or less in full control.  There are very
 few cases where we generate further trees outside the middle-end.  The only
 case I know off-hand is the innards of va_start and va_arg, which can be
 generated by the backend.

 So let's suppose we had a similar situation there, where we wanted
 va_arg do something special in a certain situation.  If we had the
 same three choices of:

  1. use an on-the-side hook to represent the special something
  2. scan the code generated by the backend and automatically
     inject the special something at an appropriate place
  3. require each backend to do it properly from the start

 (OK, slightly prejudiced wording :-)) I think we'd still choose 3.

 For this particular issue, I don't think that hook interface I'm
 proposing is more complicated than the barrier. Instead, it is easier
 for back-end implementer to be aware of the potential issue before
 really solving stack red zone problem, because it is very clearly
 listed in target hook list.

 The point for model it in the IL supporters like myself is that we
 have both many backends and many rtl passes.  Putting it in a hook keeps
 things simple for the backends, but it means that every rtl pass must be
 aware of this on-the-side dependency.  Perhaps sched2 really is the only
 pass that needs to look at the hook at present.  But perhaps not.
 E.g. dbr_schedule (not a problem on ARM, I realise) also reorders
 instructions, so maybe it would need to be audited to see whether any
 calls to this hook are needed.  And perhaps we'd add more rtl passes
 later.

 The point behind using a barrier is that the rtl passes do not then need
 to treat the stack-deallocation dependency as a special case.  They can
 just use the normal analysis and get it right.

 In other words, we're both arguing for safety here.

Indeed.  It's certainly not only scheduling that can move instructions,
but RTL PRE, combine, ifcvt all can effectively cause instruction motion
(just to name a few).

Richard.

 Richard

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

[Richard Sandiford]

Richard Sandiford richard.sandif...@linaro.org writes:
 In contrast, after the tree optimisers have handed off the initial IL,

um, I meant frontend :-)

 the tree optimisers are more or less in full control.

Richard

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

[Richard Henderson]

On 09/29/2011 06:13 PM, Jiangning Liu wrote:
 
 
 -Original Message-
 From: Jakub Jelinek [mailto:ja...@redhat.com]
 Sent: Thursday, September 29, 2011 6:14 PM
 To: Jiangning Liu
 Cc: 'Richard Guenther'; Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)

 On Thu, Sep 29, 2011 at 06:08:50PM +0800, Jiangning Liu wrote:
 As far as I know different back-ends are implementing different
 prologue/epilogue in GCC. If one day this part can be refined and
 abstracted
 as well, I would say solving this stack-red-zone problem in shared
 prologue/epilogue code would be a perfect solution, and barrier can
 be
 inserted there.

 I'm not saying you are wrong on keeping scheduler using a pure
 barrier
 interface. From engineering point of view, I only feel my proposal is
 so far
 so good, because this patch at least solve the problem for all
 targets in a
 quite simple way. Maybe it can be improved in future based on this.

 But you don't want to listen about any other alternative, other
 backends are
 happy with being able to put the best kind of barrier at the best spot
 in the epilogue and don't need a generic solution which won't model
 very
 well the target diversity anyway.
 
 Jakub,
 
 Appreciate for your attention on this issue,
 
 1) Can you clarify who are the others back-ends? Does it cover most of the
 back-ends being supported by GCC right now?

Your red-stack barrier issue is *exactly* the same as the frame pointer
barrier issue, which affects many backends.

That is, if the frame pointer is initialized before the local stack frame
is allocated, then one has to add a barrier such that memory references
based on the frame pointer are not scheduled before the local stack frame
allocation.

One example of this is in the i386 port, where the prologue looks like

push%ebp
mov %esp, %ebp
sub $frame, %esp

The rtl we emit for that subtraction looks like

(define_insn pro_epilogue_adjust_stack_mode_add
  [(set (match_operand:P 0 register_operand =r,r)
(plus:P (match_operand:P 1 register_operand 0,r)
(match_operand:P 2 nonmemory_operand ri,li)))
   (clobber (reg:CC FLAGS_REG))
   (clobber (mem:BLK (scratch)))]

Note the final clobber, which is a memory scheduling barrier.

Other targets use similar tricks.  For instance arm stack_tie.

Honestly, I've found nothing convincing throughout this thread that
suggests to me that this problem should be handled generically.


r~

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

[Richard Guenther]

On Thu, Sep 29, 2011 at 5:13 AM, Jiangning Liu jiangning@arm.com wrote:


 -Original Message-
 From: Richard Guenther [mailto:richard.guent...@gmail.com]
 Sent: Wednesday, September 28, 2011 5:56 PM
 To: Jiangning Liu
 Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)

 On Wed, Sep 28, 2011 at 11:40 AM, Jiangning Liu jiangning@arm.com
 wrote:
 
 
  -Original Message-
  From: Richard Guenther [mailto:richard.guent...@gmail.com]
  Sent: Wednesday, September 28, 2011 5:20 PM
  To: Jiangning Liu
  Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
  Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
  On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu
 jiangning@arm.com
  wrote:
  
  
   -Original Message-
   From: Richard Guenther [mailto:richard.guent...@gmail.com]
   Sent: Wednesday, September 28, 2011 4:39 PM
   To: Jiangning Liu
   Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
   Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
  
   On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu
  jiangning@arm.com
   wrote:
   
   
-Original Message-
From: Richard Guenther [mailto:richard.guent...@gmail.com]
Sent: Tuesday, September 27, 2011 3:41 PM
To: Jiangning Liu
Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
   
On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
   jiangning@arm.com
wrote:
 Think of it this way.  What the IR says is there is no
 barrier
between
 those moves.  You either have an implicit barrier (which is
  what
   you
 are proposing) or you have it explicitly.  I think we all
  rather
have
 more things explicit rather than implicit in the IR.  And
 that
   has
 been the overall feeling for a few years now.


 Sorry, I'm afraid I can't agree with you. Instead, I think
  using
barrier to describe this kind of dependence is a kind of
 implicit
method. Please note that this is not an usual data dependence
  issue.
The stack pointer change doesn't have any dependence with
 memory
   access
at all.
   
It is similar to atomic instructions that require being an
optimization/memory barrier.  Sure it is not a usual data
  dependence
(otherwise we would handle
it already), so the targets have to explicitly express the
   dependence
somehow, for which we only have barriers right now.
   
   
Richard,
   
Thanks for your explanation. It's explicit to back-end, while
 it's
   implicit
to scheduler in middle end, because barrier can decide
 dependence
  in
scheduler but barrier can be generated from several different
   scenarios.
It's unsafe and prone to introduce bug if any one of the
 scenarios
   requiring
generating barriers is being missed in back-end.
   
Between middle-end and back-end, we should have interfaces that
 is
   easy to
be implemented by back-end. After all, middle-end itself can't
   consist of a
compiler, and vice versa. Back-end needs middle-end's help to
 make
   sure
back-end is easy to be implemented and reduce the possibility
 of
   introducing
bugs.
   
Without an explicit hook as I'm proposing, back-end
 implementers
  have
   to
clearly know all scenarios of generating barrier very clearly,
   because there
isn't any code tips and comments in middle end telling back-end
  the
   list of
all scenarios on generating barriers.
   
Yes, barrier is a perfect interface for scheduler in theory.
 But
  from
engineering point of view, I think it's better to explicitly
  define
   an
interface to describe stack red zone and inform back-end, or
 vice
   versa. Not
like computer, people is easy to make mistake if you don't tell
  them.
   On
this bug, the fact is over the years different back-ends made
  similar
   bugs.
   
GCC is really a perfect platform on building new ports, and I
 saw
  a
   lot of
new back-ends. The current middle end is unsafe, if port
 doesn't
   support
stack red zone and back-ends doesn't generate barrier for it.
 Why
   can't we
explicitly clarify this in compiler code between middle end and
  back
   end?
What if any other back-end (new or old) NOT supporting stack
 red
  zone
exposing the similar bug again?
  
   There are gazillion things you have to explicitly get right in
 your
   backends,
   so I don't see why exposing proper scheduling barriers should be
   special,
   and there, why red-zones should be special (as opposed to other
   occasions
   where you need to emit barriers from the backend for the
 scheduler).
  
  
   Richard,
  
   This is because,
  
   1) Current scheduler is unsafe if back-end doesn't generate
 barrier
  for a
   port which doesn't support stack red zone at all.
   2) Implementing barrier in back-end is a burden to new back-end
   implementation for ports not supporting stack red

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

[Richard Sandiford]

Andrew Pinski pins...@gmail.com writes:
 On Mon, Sep 26, 2011 at 7:28 PM, Jiangning Liu jiangning@arm.com wrote:

 Sorry, for this bug, I don’t see your valuable comments previously in either 
 bug zilla and the [RFC] I sent out long time ago in gcc mail list. What I 
 see is a bunch of people agreed this problem should be fixed in middle end.

 The only person I see that agrees this problem should be fixed in the
 middle-end is Richard S.  Everyone else seems like is saying it should
 be fixed in the back-end.

paranoiaDid you mean Richard E?/paranoia

Richard

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

[Jiangning Liu]

 -Original Message-
 From: Richard Guenther [mailto:richard.guent...@gmail.com]
 Sent: Thursday, September 29, 2011 5:03 PM
 To: Jiangning Liu
 Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
 On Thu, Sep 29, 2011 at 5:13 AM, Jiangning Liu jiangning@arm.com
 wrote:
 
 
  -Original Message-
  From: Richard Guenther [mailto:richard.guent...@gmail.com]
  Sent: Wednesday, September 28, 2011 5:56 PM
  To: Jiangning Liu
  Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
  Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
  On Wed, Sep 28, 2011 at 11:40 AM, Jiangning Liu
 jiangning@arm.com
  wrote:
  
  
   -Original Message-
   From: Richard Guenther [mailto:richard.guent...@gmail.com]
   Sent: Wednesday, September 28, 2011 5:20 PM
   To: Jiangning Liu
   Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
   Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
  
   On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu
  jiangning@arm.com
   wrote:
   
   
-Original Message-
From: Richard Guenther [mailto:richard.guent...@gmail.com]
Sent: Wednesday, September 28, 2011 4:39 PM
To: Jiangning Liu
Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
   
On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu
   jiangning@arm.com
wrote:


 -Original Message-
 From: Richard Guenther [mailto:richard.guent...@gmail.com]
 Sent: Tuesday, September 27, 2011 3:41 PM
 To: Jiangning Liu
 Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)

 On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
jiangning@arm.com
 wrote:
  Think of it this way.  What the IR says is there is no
  barrier
 between
  those moves.  You either have an implicit barrier (which
 is
   what
you
  are proposing) or you have it explicitly.  I think we
 all
   rather
 have
  more things explicit rather than implicit in the
 IR.  And
  that
has
  been the overall feeling for a few years now.
 
 
  Sorry, I'm afraid I can't agree with you. Instead, I
 think
   using
 barrier to describe this kind of dependence is a kind of
  implicit
 method. Please note that this is not an usual data
 dependence
   issue.
 The stack pointer change doesn't have any dependence with
  memory
access
 at all.

 It is similar to atomic instructions that require being an
 optimization/memory barrier.  Sure it is not a usual data
   dependence
 (otherwise we would handle
 it already), so the targets have to explicitly express the
dependence
 somehow, for which we only have barriers right now.


 Richard,

 Thanks for your explanation. It's explicit to back-end,
 while
  it's
implicit
 to scheduler in middle end, because barrier can decide
  dependence
   in
 scheduler but barrier can be generated from several
 different
scenarios.
 It's unsafe and prone to introduce bug if any one of the
  scenarios
requiring
 generating barriers is being missed in back-end.

 Between middle-end and back-end, we should have interfaces
 that
  is
easy to
 be implemented by back-end. After all, middle-end itself
 can't
consist of a
 compiler, and vice versa. Back-end needs middle-end's help
 to
  make
sure
 back-end is easy to be implemented and reduce the
 possibility
  of
introducing
 bugs.

 Without an explicit hook as I'm proposing, back-end
  implementers
   have
to
 clearly know all scenarios of generating barrier very
 clearly,
because there
 isn't any code tips and comments in middle end telling back-
 end
   the
list of
 all scenarios on generating barriers.

 Yes, barrier is a perfect interface for scheduler in theory.
  But
   from
 engineering point of view, I think it's better to explicitly
   define
an
 interface to describe stack red zone and inform back-end, or
  vice
versa. Not
 like computer, people is easy to make mistake if you don't
 tell
   them.
On
 this bug, the fact is over the years different back-ends
 made
   similar
bugs.

 GCC is really a perfect platform on building new ports, and
 I
  saw
   a
lot of
 new back-ends. The current middle end is unsafe, if port
  doesn't
support
 stack red zone and back-ends doesn't generate barrier for it.
  Why
can't we
 explicitly clarify this in compiler code between middle end
 and
   back
end?
 What if any other back-end (new or old) NOT supporting stack
  red
   zone
 exposing the similar bug again?
   
There are gazillion things you have to explicitly get right in
  your
backends,
so I don't see why exposing proper scheduling barriers should
 be
special

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

[Jiangning Liu]

 -Original Message-
 From: Jakub Jelinek [mailto:ja...@redhat.com]
 Sent: Thursday, September 29, 2011 6:14 PM
 To: Jiangning Liu
 Cc: 'Richard Guenther'; Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
 On Thu, Sep 29, 2011 at 06:08:50PM +0800, Jiangning Liu wrote:
  As far as I know different back-ends are implementing different
  prologue/epilogue in GCC. If one day this part can be refined and
 abstracted
  as well, I would say solving this stack-red-zone problem in shared
  prologue/epilogue code would be a perfect solution, and barrier can
 be
  inserted there.
 
  I'm not saying you are wrong on keeping scheduler using a pure
 barrier
  interface. From engineering point of view, I only feel my proposal is
 so far
  so good, because this patch at least solve the problem for all
 targets in a
  quite simple way. Maybe it can be improved in future based on this.
 
 But you don't want to listen about any other alternative, other
 backends are
 happy with being able to put the best kind of barrier at the best spot
 in the epilogue and don't need a generic solution which won't model
 very
 well the target diversity anyway.

Jakub,

Appreciate for your attention on this issue,

1) Can you clarify who are the others back-ends? Does it cover most of the
back-ends being supported by GCC right now?
2) You need give data to prove other back-ends are happy with current
solution. The fact is over the years there are a bunch of bugs filed against
this problem. WHY? At this point you are implying other back-ends are
happy with bugs or potential bugs? This is wired to me. Also, this is not a
issue whether a back-end is able to implement barrier or not. If you are
really asking able or not, I would say every back-end is able, but it
doesn't mean able is correct and it doesn't mean able is the most
reasonable.

Comparing with the one I am proposing, I don't see the current solution has
other significant advantages in addition to the proper modeling for
scheduler you are arguing. Instead, the solution I'm proposing is a safe
solution, and a solution easy to avoid bugs. If GCC compiler infrastructure
can't even give a safe compilation, why should we insist on the proper
modeling for scheduler only?

Hopefully you can consider again about this.

-Jiangning

 
   Jakub

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

[Jiangning Liu]

  -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 

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

[Jiangning Liu]

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-ow...@gcc.gnu.org [mailto:gcc-patches-
 ow...@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; j...@suse.cz; geo...@geoffk.org;
 dje@gmail.com; r...@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

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

[Uros Bizjak]

On Wed, Sep 28, 2011 at 8:42 AM, Jiangning Liu jiangning@arm.com wrote:
 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.

OK for x86, but you need approval from middle-end maintainer for the patch.

Thanks,
Uros.

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

[Richard Guenther]

On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu jiangning@arm.com wrote:


 -Original Message-
 From: Richard Guenther [mailto:richard.guent...@gmail.com]
 Sent: Tuesday, September 27, 2011 3:41 PM
 To: Jiangning Liu
 Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)

 On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu jiangning@arm.com
 wrote:
  Think of it this way.  What the IR says is there is no barrier
 between
  those moves.  You either have an implicit barrier (which is what you
  are proposing) or you have it explicitly.  I think we all rather
 have
  more things explicit rather than implicit in the IR.  And that has
  been the overall feeling for a few years now.
 
 
  Sorry, I'm afraid I can't agree with you. Instead, I think using
 barrier to describe this kind of dependence is a kind of implicit
 method. Please note that this is not an usual data dependence issue.
 The stack pointer change doesn't have any dependence with memory access
 at all.

 It is similar to atomic instructions that require being an
 optimization/memory barrier.  Sure it is not a usual data dependence
 (otherwise we would handle
 it already), so the targets have to explicitly express the dependence
 somehow, for which we only have barriers right now.


 Richard,

 Thanks for your explanation. It's explicit to back-end, while it's implicit
 to scheduler in middle end, because barrier can decide dependence in
 scheduler but barrier can be generated from several different scenarios.
 It's unsafe and prone to introduce bug if any one of the scenarios requiring
 generating barriers is being missed in back-end.

 Between middle-end and back-end, we should have interfaces that is easy to
 be implemented by back-end. After all, middle-end itself can't consist of a
 compiler, and vice versa. Back-end needs middle-end's help to make sure
 back-end is easy to be implemented and reduce the possibility of introducing
 bugs.

 Without an explicit hook as I'm proposing, back-end implementers have to
 clearly know all scenarios of generating barrier very clearly, because there
 isn't any code tips and comments in middle end telling back-end the list of
 all scenarios on generating barriers.

 Yes, barrier is a perfect interface for scheduler in theory. But from
 engineering point of view, I think it's better to explicitly define an
 interface to describe stack red zone and inform back-end, or vice versa. Not
 like computer, people is easy to make mistake if you don't tell them. On
 this bug, the fact is over the years different back-ends made similar bugs.

 GCC is really a perfect platform on building new ports, and I saw a lot of
 new back-ends. The current middle end is unsafe, if port doesn't support
 stack red zone and back-ends doesn't generate barrier for it. Why can't we
 explicitly clarify this in compiler code between middle end and back end?
 What if any other back-end (new or old) NOT supporting stack red zone
 exposing the similar bug again?

There are gazillion things you have to explicitly get right in your backends,
so I don't see why exposing proper scheduling barriers should be special,
and there, why red-zones should be special (as opposed to other occasions
where you need to emit barriers from the backend for the scheduler).

Richard.

 Thanks,
 -Jiangning

 Richard.

  No matter what solution itself is, the problem itself is a quite a
 common one on ISA level, so we should solve it in middle-end, because
 middle-end is shared for all ports.
 
  My proposal avoids problems in future. Any new ports and new back-end
 implementations needn't explicitly define this hook in a very safe way.
 But if one port wants to unsafely introduce red zone, we can
 explicitly define this hook in back-end. This way, we would avoid the
 bug in the earliest time. Do you really want to hide this problem in
 back-end silently? And wait others to report bug and then waste time to
 get it fixed again?
 
  The facts I see is over the years, for different ports reported the
 similar issue around this, and somebody tried to fix them. Actually,
 this kind of problem should be fixed in design level. If the first
 people solve this bug can give solution in middle end, we would not
 need to waste time on filing those bugs in bug zilla and fixing them
 around this problem.
 
  Thanks,
  -Jiangning
 
 
 
 
 
 
 
 

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

[Jiangning Liu]

 -Original Message-
 From: Richard Guenther [mailto:richard.guent...@gmail.com]
 Sent: Wednesday, September 28, 2011 4:39 PM
 To: Jiangning Liu
 Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
 On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu jiangning@arm.com
 wrote:
 
 
  -Original Message-
  From: Richard Guenther [mailto:richard.guent...@gmail.com]
  Sent: Tuesday, September 27, 2011 3:41 PM
  To: Jiangning Liu
  Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
  Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
  On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
 jiangning@arm.com
  wrote:
   Think of it this way.  What the IR says is there is no barrier
  between
   those moves.  You either have an implicit barrier (which is what
 you
   are proposing) or you have it explicitly.  I think we all rather
  have
   more things explicit rather than implicit in the IR.  And that
 has
   been the overall feeling for a few years now.
  
  
   Sorry, I'm afraid I can't agree with you. Instead, I think using
  barrier to describe this kind of dependence is a kind of implicit
  method. Please note that this is not an usual data dependence issue.
  The stack pointer change doesn't have any dependence with memory
 access
  at all.
 
  It is similar to atomic instructions that require being an
  optimization/memory barrier.  Sure it is not a usual data dependence
  (otherwise we would handle
  it already), so the targets have to explicitly express the
 dependence
  somehow, for which we only have barriers right now.
 
 
  Richard,
 
  Thanks for your explanation. It's explicit to back-end, while it's
 implicit
  to scheduler in middle end, because barrier can decide dependence in
  scheduler but barrier can be generated from several different
 scenarios.
  It's unsafe and prone to introduce bug if any one of the scenarios
 requiring
  generating barriers is being missed in back-end.
 
  Between middle-end and back-end, we should have interfaces that is
 easy to
  be implemented by back-end. After all, middle-end itself can't
 consist of a
  compiler, and vice versa. Back-end needs middle-end's help to make
 sure
  back-end is easy to be implemented and reduce the possibility of
 introducing
  bugs.
 
  Without an explicit hook as I'm proposing, back-end implementers have
 to
  clearly know all scenarios of generating barrier very clearly,
 because there
  isn't any code tips and comments in middle end telling back-end the
 list of
  all scenarios on generating barriers.
 
  Yes, barrier is a perfect interface for scheduler in theory. But from
  engineering point of view, I think it's better to explicitly define
 an
  interface to describe stack red zone and inform back-end, or vice
 versa. Not
  like computer, people is easy to make mistake if you don't tell them.
 On
  this bug, the fact is over the years different back-ends made similar
 bugs.
 
  GCC is really a perfect platform on building new ports, and I saw a
 lot of
  new back-ends. The current middle end is unsafe, if port doesn't
 support
  stack red zone and back-ends doesn't generate barrier for it. Why
 can't we
  explicitly clarify this in compiler code between middle end and back
 end?
  What if any other back-end (new or old) NOT supporting stack red zone
  exposing the similar bug again?
 
 There are gazillion things you have to explicitly get right in your
 backends,
 so I don't see why exposing proper scheduling barriers should be
 special,
 and there, why red-zones should be special (as opposed to other
 occasions
 where you need to emit barriers from the backend for the scheduler).
 

Richard,

This is because,

1) Current scheduler is unsafe if back-end doesn't generate barrier for a
port which doesn't support stack red zone at all.
2) Implementing barrier in back-end is a burden to new back-end
implementation for ports not supporting stack red zone at all. 
3) There are many other ports not reporting bugs around this. I doubt there
isn't bug for them. 
4) There are over 300 TARGET HOOKS being defined in target.def. I don't
think adding this interface is hurting GCC.

BTW, really appreciate your close attention to this specific issue.

Thanks,
-Jiangning

 Richard.
 
  Thanks,
  -Jiangning
 
  Richard.
 
   No matter what solution itself is, the problem itself is a quite a
  common one on ISA level, so we should solve it in middle-end,
 because
  middle-end is shared for all ports.
  
   My proposal avoids problems in future. Any new ports and new back-
 end
  implementations needn't explicitly define this hook in a very safe
 way.
  But if one port wants to unsafely introduce red zone, we can
  explicitly define this hook in back-end. This way, we would avoid
 the
  bug in the earliest time. Do you really want to hide this problem in
  back-end silently? And wait others to report bug and then waste time
 to
  get it fixed again?
  
   The facts I see is over the years

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

[Richard Guenther]

On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu jiangning@arm.com wrote:


 -Original Message-
 From: Richard Guenther [mailto:richard.guent...@gmail.com]
 Sent: Wednesday, September 28, 2011 4:39 PM
 To: Jiangning Liu
 Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)

 On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu jiangning@arm.com
 wrote:
 
 
  -Original Message-
  From: Richard Guenther [mailto:richard.guent...@gmail.com]
  Sent: Tuesday, September 27, 2011 3:41 PM
  To: Jiangning Liu
  Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
  Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
  On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
 jiangning@arm.com
  wrote:
   Think of it this way.  What the IR says is there is no barrier
  between
   those moves.  You either have an implicit barrier (which is what
 you
   are proposing) or you have it explicitly.  I think we all rather
  have
   more things explicit rather than implicit in the IR.  And that
 has
   been the overall feeling for a few years now.
  
  
   Sorry, I'm afraid I can't agree with you. Instead, I think using
  barrier to describe this kind of dependence is a kind of implicit
  method. Please note that this is not an usual data dependence issue.
  The stack pointer change doesn't have any dependence with memory
 access
  at all.
 
  It is similar to atomic instructions that require being an
  optimization/memory barrier.  Sure it is not a usual data dependence
  (otherwise we would handle
  it already), so the targets have to explicitly express the
 dependence
  somehow, for which we only have barriers right now.
 
 
  Richard,
 
  Thanks for your explanation. It's explicit to back-end, while it's
 implicit
  to scheduler in middle end, because barrier can decide dependence in
  scheduler but barrier can be generated from several different
 scenarios.
  It's unsafe and prone to introduce bug if any one of the scenarios
 requiring
  generating barriers is being missed in back-end.
 
  Between middle-end and back-end, we should have interfaces that is
 easy to
  be implemented by back-end. After all, middle-end itself can't
 consist of a
  compiler, and vice versa. Back-end needs middle-end's help to make
 sure
  back-end is easy to be implemented and reduce the possibility of
 introducing
  bugs.
 
  Without an explicit hook as I'm proposing, back-end implementers have
 to
  clearly know all scenarios of generating barrier very clearly,
 because there
  isn't any code tips and comments in middle end telling back-end the
 list of
  all scenarios on generating barriers.
 
  Yes, barrier is a perfect interface for scheduler in theory. But from
  engineering point of view, I think it's better to explicitly define
 an
  interface to describe stack red zone and inform back-end, or vice
 versa. Not
  like computer, people is easy to make mistake if you don't tell them.
 On
  this bug, the fact is over the years different back-ends made similar
 bugs.
 
  GCC is really a perfect platform on building new ports, and I saw a
 lot of
  new back-ends. The current middle end is unsafe, if port doesn't
 support
  stack red zone and back-ends doesn't generate barrier for it. Why
 can't we
  explicitly clarify this in compiler code between middle end and back
 end?
  What if any other back-end (new or old) NOT supporting stack red zone
  exposing the similar bug again?

 There are gazillion things you have to explicitly get right in your
 backends,
 so I don't see why exposing proper scheduling barriers should be
 special,
 and there, why red-zones should be special (as opposed to other
 occasions
 where you need to emit barriers from the backend for the scheduler).


 Richard,

 This is because,

 1) Current scheduler is unsafe if back-end doesn't generate barrier for a
 port which doesn't support stack red zone at all.
 2) Implementing barrier in back-end is a burden to new back-end
 implementation for ports not supporting stack red zone at all.
 3) There are many other ports not reporting bugs around this. I doubt there
 isn't bug for them.
 4) There are over 300 TARGET HOOKS being defined in target.def. I don't
 think adding this interface is hurting GCC.

I don't argue that your solution is not acceptable, just your reasoning
is bogus IMHO. 1) is a target bug, 2) huh, I doubt that this is the
biggest issue
one faces when implementing a new target, 3) I'm sure there are very many
latent backend bugs not related to red-zone

The middle-end isn't safe by default either if you have bogus
instruction patterns in your .md file, or you generate bogus IL
from the va-arg gimplification hook.  A target bug is a target bug,
the middle-end can't and should not do anything to try to workaround
bugs in targets.

 BTW, really appreciate your close attention to this specific issue.

 Thanks,
 -Jiangning

 Richard.

  Thanks,
  -Jiangning
 
  Richard.
 
   No matter what solution itself

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

[Jiangning Liu]

 -Original Message-
 From: Richard Guenther [mailto:richard.guent...@gmail.com]
 Sent: Wednesday, September 28, 2011 5:20 PM
 To: Jiangning Liu
 Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
 On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu jiangning@arm.com
 wrote:
 
 
  -Original Message-
  From: Richard Guenther [mailto:richard.guent...@gmail.com]
  Sent: Wednesday, September 28, 2011 4:39 PM
  To: Jiangning Liu
  Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
  Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
  On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu
 jiangning@arm.com
  wrote:
  
  
   -Original Message-
   From: Richard Guenther [mailto:richard.guent...@gmail.com]
   Sent: Tuesday, September 27, 2011 3:41 PM
   To: Jiangning Liu
   Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
   Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
  
   On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
  jiangning@arm.com
   wrote:
Think of it this way.  What the IR says is there is no barrier
   between
those moves.  You either have an implicit barrier (which is
 what
  you
are proposing) or you have it explicitly.  I think we all
 rather
   have
more things explicit rather than implicit in the IR.  And that
  has
been the overall feeling for a few years now.
   
   
Sorry, I'm afraid I can't agree with you. Instead, I think
 using
   barrier to describe this kind of dependence is a kind of implicit
   method. Please note that this is not an usual data dependence
 issue.
   The stack pointer change doesn't have any dependence with memory
  access
   at all.
  
   It is similar to atomic instructions that require being an
   optimization/memory barrier.  Sure it is not a usual data
 dependence
   (otherwise we would handle
   it already), so the targets have to explicitly express the
  dependence
   somehow, for which we only have barriers right now.
  
  
   Richard,
  
   Thanks for your explanation. It's explicit to back-end, while it's
  implicit
   to scheduler in middle end, because barrier can decide dependence
 in
   scheduler but barrier can be generated from several different
  scenarios.
   It's unsafe and prone to introduce bug if any one of the scenarios
  requiring
   generating barriers is being missed in back-end.
  
   Between middle-end and back-end, we should have interfaces that is
  easy to
   be implemented by back-end. After all, middle-end itself can't
  consist of a
   compiler, and vice versa. Back-end needs middle-end's help to make
  sure
   back-end is easy to be implemented and reduce the possibility of
  introducing
   bugs.
  
   Without an explicit hook as I'm proposing, back-end implementers
 have
  to
   clearly know all scenarios of generating barrier very clearly,
  because there
   isn't any code tips and comments in middle end telling back-end
 the
  list of
   all scenarios on generating barriers.
  
   Yes, barrier is a perfect interface for scheduler in theory. But
 from
   engineering point of view, I think it's better to explicitly
 define
  an
   interface to describe stack red zone and inform back-end, or vice
  versa. Not
   like computer, people is easy to make mistake if you don't tell
 them.
  On
   this bug, the fact is over the years different back-ends made
 similar
  bugs.
  
   GCC is really a perfect platform on building new ports, and I saw
 a
  lot of
   new back-ends. The current middle end is unsafe, if port doesn't
  support
   stack red zone and back-ends doesn't generate barrier for it. Why
  can't we
   explicitly clarify this in compiler code between middle end and
 back
  end?
   What if any other back-end (new or old) NOT supporting stack red
 zone
   exposing the similar bug again?
 
  There are gazillion things you have to explicitly get right in your
  backends,
  so I don't see why exposing proper scheduling barriers should be
  special,
  and there, why red-zones should be special (as opposed to other
  occasions
  where you need to emit barriers from the backend for the scheduler).
 
 
  Richard,
 
  This is because,
 
  1) Current scheduler is unsafe if back-end doesn't generate barrier
 for a
  port which doesn't support stack red zone at all.
  2) Implementing barrier in back-end is a burden to new back-end
  implementation for ports not supporting stack red zone at all.
  3) There are many other ports not reporting bugs around this. I doubt
 there
  isn't bug for them.
  4) There are over 300 TARGET HOOKS being defined in target.def. I
 don't
  think adding this interface is hurting GCC.
 
 I don't argue that your solution is not acceptable, just your reasoning
 is bogus IMHO. 
 1) is a target bug, 

Why should back-ends handle a thing that doesn't exist at all for them? I
don't see clear logic here. 

 2) huh, I doubt that this is the
 biggest issue
 one faces when implementing a new target, 

I never say

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

[Richard Guenther]

On Wed, Sep 28, 2011 at 11:40 AM, Jiangning Liu jiangning@arm.com wrote:


 -Original Message-
 From: Richard Guenther [mailto:richard.guent...@gmail.com]
 Sent: Wednesday, September 28, 2011 5:20 PM
 To: Jiangning Liu
 Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)

 On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu jiangning@arm.com
 wrote:
 
 
  -Original Message-
  From: Richard Guenther [mailto:richard.guent...@gmail.com]
  Sent: Wednesday, September 28, 2011 4:39 PM
  To: Jiangning Liu
  Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
  Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
  On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu
 jiangning@arm.com
  wrote:
  
  
   -Original Message-
   From: Richard Guenther [mailto:richard.guent...@gmail.com]
   Sent: Tuesday, September 27, 2011 3:41 PM
   To: Jiangning Liu
   Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
   Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
  
   On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
  jiangning@arm.com
   wrote:
Think of it this way.  What the IR says is there is no barrier
   between
those moves.  You either have an implicit barrier (which is
 what
  you
are proposing) or you have it explicitly.  I think we all
 rather
   have
more things explicit rather than implicit in the IR.  And that
  has
been the overall feeling for a few years now.
   
   
Sorry, I'm afraid I can't agree with you. Instead, I think
 using
   barrier to describe this kind of dependence is a kind of implicit
   method. Please note that this is not an usual data dependence
 issue.
   The stack pointer change doesn't have any dependence with memory
  access
   at all.
  
   It is similar to atomic instructions that require being an
   optimization/memory barrier.  Sure it is not a usual data
 dependence
   (otherwise we would handle
   it already), so the targets have to explicitly express the
  dependence
   somehow, for which we only have barriers right now.
  
  
   Richard,
  
   Thanks for your explanation. It's explicit to back-end, while it's
  implicit
   to scheduler in middle end, because barrier can decide dependence
 in
   scheduler but barrier can be generated from several different
  scenarios.
   It's unsafe and prone to introduce bug if any one of the scenarios
  requiring
   generating barriers is being missed in back-end.
  
   Between middle-end and back-end, we should have interfaces that is
  easy to
   be implemented by back-end. After all, middle-end itself can't
  consist of a
   compiler, and vice versa. Back-end needs middle-end's help to make
  sure
   back-end is easy to be implemented and reduce the possibility of
  introducing
   bugs.
  
   Without an explicit hook as I'm proposing, back-end implementers
 have
  to
   clearly know all scenarios of generating barrier very clearly,
  because there
   isn't any code tips and comments in middle end telling back-end
 the
  list of
   all scenarios on generating barriers.
  
   Yes, barrier is a perfect interface for scheduler in theory. But
 from
   engineering point of view, I think it's better to explicitly
 define
  an
   interface to describe stack red zone and inform back-end, or vice
  versa. Not
   like computer, people is easy to make mistake if you don't tell
 them.
  On
   this bug, the fact is over the years different back-ends made
 similar
  bugs.
  
   GCC is really a perfect platform on building new ports, and I saw
 a
  lot of
   new back-ends. The current middle end is unsafe, if port doesn't
  support
   stack red zone and back-ends doesn't generate barrier for it. Why
  can't we
   explicitly clarify this in compiler code between middle end and
 back
  end?
   What if any other back-end (new or old) NOT supporting stack red
 zone
   exposing the similar bug again?
 
  There are gazillion things you have to explicitly get right in your
  backends,
  so I don't see why exposing proper scheduling barriers should be
  special,
  and there, why red-zones should be special (as opposed to other
  occasions
  where you need to emit barriers from the backend for the scheduler).
 
 
  Richard,
 
  This is because,
 
  1) Current scheduler is unsafe if back-end doesn't generate barrier
 for a
  port which doesn't support stack red zone at all.
  2) Implementing barrier in back-end is a burden to new back-end
  implementation for ports not supporting stack red zone at all.
  3) There are many other ports not reporting bugs around this. I doubt
 there
  isn't bug for them.
  4) There are over 300 TARGET HOOKS being defined in target.def. I
 don't
  think adding this interface is hurting GCC.

 I don't argue that your solution is not acceptable, just your reasoning
 is bogus IMHO.
 1) is a target bug,

 Why should back-ends handle a thing that doesn't exist at all for them? I
 don't see clear logic here.

I don't think this is the case here.  You need

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

[Jiangning Liu]

 -Original Message-
 From: Richard Guenther [mailto:richard.guent...@gmail.com]
 Sent: Wednesday, September 28, 2011 5:56 PM
 To: Jiangning Liu
 Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
 On Wed, Sep 28, 2011 at 11:40 AM, Jiangning Liu jiangning@arm.com
 wrote:
 
 
  -Original Message-
  From: Richard Guenther [mailto:richard.guent...@gmail.com]
  Sent: Wednesday, September 28, 2011 5:20 PM
  To: Jiangning Liu
  Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
  Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
  On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu
 jiangning@arm.com
  wrote:
  
  
   -Original Message-
   From: Richard Guenther [mailto:richard.guent...@gmail.com]
   Sent: Wednesday, September 28, 2011 4:39 PM
   To: Jiangning Liu
   Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
   Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
  
   On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu
  jiangning@arm.com
   wrote:
   
   
-Original Message-
From: Richard Guenther [mailto:richard.guent...@gmail.com]
Sent: Tuesday, September 27, 2011 3:41 PM
To: Jiangning Liu
Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
   
On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
   jiangning@arm.com
wrote:
 Think of it this way.  What the IR says is there is no
 barrier
between
 those moves.  You either have an implicit barrier (which is
  what
   you
 are proposing) or you have it explicitly.  I think we all
  rather
have
 more things explicit rather than implicit in the IR.  And
 that
   has
 been the overall feeling for a few years now.


 Sorry, I'm afraid I can't agree with you. Instead, I think
  using
barrier to describe this kind of dependence is a kind of
 implicit
method. Please note that this is not an usual data dependence
  issue.
The stack pointer change doesn't have any dependence with
 memory
   access
at all.
   
It is similar to atomic instructions that require being an
optimization/memory barrier.  Sure it is not a usual data
  dependence
(otherwise we would handle
it already), so the targets have to explicitly express the
   dependence
somehow, for which we only have barriers right now.
   
   
Richard,
   
Thanks for your explanation. It's explicit to back-end, while
 it's
   implicit
to scheduler in middle end, because barrier can decide
 dependence
  in
scheduler but barrier can be generated from several different
   scenarios.
It's unsafe and prone to introduce bug if any one of the
 scenarios
   requiring
generating barriers is being missed in back-end.
   
Between middle-end and back-end, we should have interfaces that
 is
   easy to
be implemented by back-end. After all, middle-end itself can't
   consist of a
compiler, and vice versa. Back-end needs middle-end's help to
 make
   sure
back-end is easy to be implemented and reduce the possibility
 of
   introducing
bugs.
   
Without an explicit hook as I'm proposing, back-end
 implementers
  have
   to
clearly know all scenarios of generating barrier very clearly,
   because there
isn't any code tips and comments in middle end telling back-end
  the
   list of
all scenarios on generating barriers.
   
Yes, barrier is a perfect interface for scheduler in theory.
 But
  from
engineering point of view, I think it's better to explicitly
  define
   an
interface to describe stack red zone and inform back-end, or
 vice
   versa. Not
like computer, people is easy to make mistake if you don't tell
  them.
   On
this bug, the fact is over the years different back-ends made
  similar
   bugs.
   
GCC is really a perfect platform on building new ports, and I
 saw
  a
   lot of
new back-ends. The current middle end is unsafe, if port
 doesn't
   support
stack red zone and back-ends doesn't generate barrier for it.
 Why
   can't we
explicitly clarify this in compiler code between middle end and
  back
   end?
What if any other back-end (new or old) NOT supporting stack
 red
  zone
exposing the similar bug again?
  
   There are gazillion things you have to explicitly get right in
 your
   backends,
   so I don't see why exposing proper scheduling barriers should be
   special,
   and there, why red-zones should be special (as opposed to other
   occasions
   where you need to emit barriers from the backend for the
 scheduler).
  
  
   Richard,
  
   This is because,
  
   1) Current scheduler is unsafe if back-end doesn't generate
 barrier
  for a
   port which doesn't support stack red zone at all.
   2) Implementing barrier in back-end is a burden to new back-end
   implementation for ports not supporting stack red zone at all.
   3) There are many other ports not reporting bugs around

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

[Uros Bizjak]

On Tue, Sep 27, 2011 at 4:36 AM, Jiangning Liu jiangning@arm.com wrote:
 Fix a typo and CC x86/rs6000/arm ports maintainers.

 ChangeLog:

        * config/i386/i386.c (ix86_stack_using_red_zone): Change inline
        to be extern.
        (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.

 Thanks,
 -Jiangning

 diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
 index ff8c49f..1c16391 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
 +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

 +#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..411cb09 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.  */
 +extern bool

static bool

 +rs6000_stack_using_red_zone (void)
 +{
 +   return (DEFAULT_ABI != ABI_V4);
 +}
 +

Uros.

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

[Richard Guenther]

On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu jiangning@arm.com wrote:
 Think of it this way.  What the IR says is there is no barrier between
 those moves.  You either have an implicit barrier (which is what you
 are proposing) or you have it explicitly.  I think we all rather have
 more things explicit rather than implicit in the IR.  And that has
 been the overall feeling for a few years now.


 Sorry, I'm afraid I can't agree with you. Instead, I think using barrier to 
 describe this kind of dependence is a kind of implicit method. Please note 
 that this is not an usual data dependence issue. The stack pointer change 
 doesn't have any dependence with memory access at all.

It is similar to atomic instructions that require being an
optimization/memory barrier.  Sure it is not a usual data dependence
(otherwise we would handle
it already), so the targets have to explicitly express the dependence
somehow, for which we only have barriers right now.

Richard.

 No matter what solution itself is, the problem itself is a quite a common one 
 on ISA level, so we should solve it in middle-end, because middle-end is 
 shared for all ports.

 My proposal avoids problems in future. Any new ports and new back-end 
 implementations needn't explicitly define this hook in a very safe way. But 
 if one port wants to unsafely introduce red zone, we can explicitly define 
 this hook in back-end. This way, we would avoid the bug in the earliest time. 
 Do you really want to hide this problem in back-end silently? And wait others 
 to report bug and then waste time to get it fixed again?

 The facts I see is over the years, for different ports reported the similar 
 issue around this, and somebody tried to fix them. Actually, this kind of 
 problem should be fixed in design level. If the first people solve this bug 
 can give solution in middle end, we would not need to waste time on filing 
 those bugs in bug zilla and fixing them around this problem.

 Thanks,
 -Jiangning

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

[Jiangning Liu]

 -Original Message-
 From: Richard Guenther [mailto:richard.guent...@gmail.com]
 Sent: Tuesday, September 27, 2011 3:41 PM
 To: Jiangning Liu
 Cc: Andrew Pinski; gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
 On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu jiangning@arm.com
 wrote:
  Think of it this way.  What the IR says is there is no barrier
 between
  those moves.  You either have an implicit barrier (which is what you
  are proposing) or you have it explicitly.  I think we all rather
 have
  more things explicit rather than implicit in the IR.  And that has
  been the overall feeling for a few years now.
 
 
  Sorry, I'm afraid I can't agree with you. Instead, I think using
 barrier to describe this kind of dependence is a kind of implicit
 method. Please note that this is not an usual data dependence issue.
 The stack pointer change doesn't have any dependence with memory access
 at all.
 
 It is similar to atomic instructions that require being an
 optimization/memory barrier.  Sure it is not a usual data dependence
 (otherwise we would handle
 it already), so the targets have to explicitly express the dependence
 somehow, for which we only have barriers right now.
 

Richard,

Thanks for your explanation. It's explicit to back-end, while it's implicit
to scheduler in middle end, because barrier can decide dependence in
scheduler but barrier can be generated from several different scenarios.
It's unsafe and prone to introduce bug if any one of the scenarios requiring
generating barriers is being missed in back-end.

Between middle-end and back-end, we should have interfaces that is easy to
be implemented by back-end. After all, middle-end itself can't consist of a
compiler, and vice versa. Back-end needs middle-end's help to make sure
back-end is easy to be implemented and reduce the possibility of introducing
bugs.

Without an explicit hook as I'm proposing, back-end implementers have to
clearly know all scenarios of generating barrier very clearly, because there
isn't any code tips and comments in middle end telling back-end the list of
all scenarios on generating barriers. 

Yes, barrier is a perfect interface for scheduler in theory. But from
engineering point of view, I think it's better to explicitly define an
interface to describe stack red zone and inform back-end, or vice versa. Not
like computer, people is easy to make mistake if you don't tell them. On
this bug, the fact is over the years different back-ends made similar bugs.

GCC is really a perfect platform on building new ports, and I saw a lot of
new back-ends. The current middle end is unsafe, if port doesn't support
stack red zone and back-ends doesn't generate barrier for it. Why can't we
explicitly clarify this in compiler code between middle end and back end?
What if any other back-end (new or old) NOT supporting stack red zone
exposing the similar bug again?

Thanks,
-Jiangning

 Richard.
 
  No matter what solution itself is, the problem itself is a quite a
 common one on ISA level, so we should solve it in middle-end, because
 middle-end is shared for all ports.
 
  My proposal avoids problems in future. Any new ports and new back-end
 implementations needn't explicitly define this hook in a very safe way.
 But if one port wants to unsafely introduce red zone, we can
 explicitly define this hook in back-end. This way, we would avoid the
 bug in the earliest time. Do you really want to hide this problem in
 back-end silently? And wait others to report bug and then waste time to
 get it fixed again?
 
  The facts I see is over the years, for different ports reported the
 similar issue around this, and somebody tried to fix them. Actually,
 this kind of problem should be fixed in design level. If the first
 people solve this bug can give solution in middle end, we would not
 need to waste time on filing those bugs in bug zilla and fixing them
 around this problem.
 
  Thanks,
  -Jiangning
 
 
 
 
 
 
 
 

[PATCH] Fix stack red zone bug (PR38644)

[Jiangning Liu]

This patch is fix PR38644, a 3-year-old bug. 

From the discussions in mail list and bugzilla, I think the middle end fix
is a common view. Although there are stills some gaps on how to fix it in
middle end, I think this patch at least moves the problem from back-end to
middle-end, which makes GCC infrastructure stronger than before. Otherwise,
any back-end needs to find and fix this problem by itself.

Since this bug was pinged many times by customers, I think at least we can
move on with this patch. If necessary, we can then give follow-up to build a
even better solution in middle end later on.

The patch is tested on x86 and ARM.

ChangeLog:

* config/i386/i386.c (ix86_stack_using_red_zone): Change inline
to be extern.
(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.

Thanks,
-Jiangning

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ff8c49f..ce486da 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2631,8 +2631,8 @@ static const char *const
cpu_names[TARGET_CPU_DEFAULT_max] =
 

 /* Return true if a red-zone is in use.  */
 
-static inline bool
-ix86_using_red_zone (void)
+extern bool
+ix86_stack_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_stack_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..411cb09 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.  */
+extern 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
+++ 

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

[Andrew Pinski]

On Mon, Sep 26, 2011 at 3:26 AM, Jiangning Liu jiangning@arm.com wrote:
 This patch is fix PR38644, a 3-year-old bug.

 From the discussions in mail list and bugzilla, I think the middle end fix
 is a common view. Although there are stills some gaps on how to fix it in
 middle end, I think this patch at least moves the problem from back-end to
 middle-end, which makes GCC infrastructure stronger than before. Otherwise,
 any back-end needs to find and fix this problem by itself.

I don't see why you think that is the common view that the fix should
be in the middle-end.  I and a few others think the back-end should be
where the barrier emitted from.  The middle-end should not have stack
accesses as being special in anyway when it comes down to scheduling.
What happens when a new scheduler comes around?  Then this has to be
fixed again in that new scheduler rather than having the barrier doing
the correct thing for all schedulers.

Thanks,
Andrew Pinski

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

[Jiangning Liu]

 -Original Message-
 From: Andrew Pinski [mailto:pins...@gmail.com]
 Sent: Tuesday, September 27, 2011 5:31 AM
 To: Jiangning Liu
 Cc: gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
 
 On Mon, Sep 26, 2011 at 3:26 AM, Jiangning Liu jiangning@arm.com
 wrote:
  This patch is fix PR38644, a 3-year-old bug.
 
  From the discussions in mail list and bugzilla, I think the middle
 end fix
  is a common view. Although there are stills some gaps on how to fix
 it in
  middle end, I think this patch at least moves the problem from back-
 end to
  middle-end, which makes GCC infrastructure stronger than before.
 Otherwise,
  any back-end needs to find and fix this problem by itself.
 
 I don't see why you think that is the common view that the fix should
 be in the middle-end.  I and a few others think the back-end should be
 where the barrier emitted from.  The middle-end should not have stack
 accesses as being special in anyway when it comes down to scheduling.
 What happens when a new scheduler comes around?  Then this has to be
 fixed again in that new scheduler rather than having the barrier doing
 the correct thing for all schedulers.
 

Hi Andrew,

Thanks for your kindly response!

Sorry, for this bug, I don’t see your valuable comments previously in either 
bug zilla and the [RFC] I sent out long time ago in gcc mail list. What I see 
is a bunch of people agreed this problem should be fixed in middle end.

For example, 
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38644#c48
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38644#c49

My comments,
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38644#c35

I'd like to repeat my points here.

As I mentioned, it shouldn't be back-end's responsibility to find this 
problem, i.e. the instructions move over stack pointer change. ISAs can be 
split into two classes. One class doesn't support stack red zone, and the other 
class does support stack red zone. If we agree this is a general issue, I think 
this issue should be solved in middle end rather than in back-end.

Yes. Back-end can provide barrier to explicitly represent the dependence, but I 
think this is a kind of workaround, because this way we are hoping back-end to 
detect this kind of dependence, while this kind of dependence is common for 
every back-end which doesn't support stack red zone. If we carefully analyze 
the implementation in x86 and powerpc back-ends supporting stack red zone, we 
may find, they are doing almost the same things.

In particular, I think the retarget-ability is the most valuable treasure for 
GCC. Thinking of implementing a new port, do we want to write new code to 
find this problem, no matter whether the new port supports stack red zone or 
not? If the answer is YES, it means we need to write the similar code like what 
currently x86-64 and powerpc back-ends are doing. Obviously, it is a 
non-trivial work. This way I don't think it's good for GCC's retarget-ability. 
Why don't we abstract the common thing in middle-end with a very clean 
interface?

Finally, It's OK for me if you say scheduler can only tread dependence as a 
clean interface. But this point doesn't support stack red zone issue must be 
handled in different back-ends respectively. If we want to keep scheduler clean 
enough, a simpler solution is adding a pass in middle-end to generate barrier 
before scheduler and this pass handle the general stack-red-zone issue using 
the interface I'm defining in the patch, but obviously this is a kind of 
over-design so far. 

Thanks,
-Jiangning

 Thanks,
 Andrew Pinski

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

[Jiangning Liu]

Fix a typo and CC x86/rs6000/arm ports maintainers.

ChangeLog:

* config/i386/i386.c (ix86_stack_using_red_zone): Change inline
to be extern.
(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.

Thanks,
-Jiangning

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ff8c49f..1c16391 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
+extern 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..411cb09 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.  */
+extern 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 

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

[Andrew Pinski]

On Mon, Sep 26, 2011 at 7:28 PM, Jiangning Liu jiangning@arm.com wrote:

 Sorry, for this bug, I don’t see your valuable comments previously in either 
 bug zilla and the [RFC] I sent out long time ago in gcc mail list. What I see 
 is a bunch of people agreed this problem should be fixed in middle end.

The only person I see that agrees this problem should be fixed in the
middle-end is Richard S.  Everyone else seems like is saying it should
be fixed in the back-end.

I mentioned in the PowerPC related bug
(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30282#c1 and it was
mentioned also by Jim Wilson in that bug report). Richard Guenther and
Jeff Law both agree with me too.  The PowerPC patch is simple to fix
the bug there is simple.  The ARM patch as suggested by Jim Wilson in
that bug report seems like the correct way forward.

Think of it this way.  What the IR says is there is no barrier between
those moves.  You either have an implicit barrier (which is what you
are proposing) or you have it explicitly.  I think we all rather have
more things explicit rather than implicit in the IR.  And that has
been the overall feeling for a few years now.

Thanks,
Andrew Pinski

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

[Jiangning Liu]

 Think of it this way.  What the IR says is there is no barrier between
 those moves.  You either have an implicit barrier (which is what you
 are proposing) or you have it explicitly.  I think we all rather have
 more things explicit rather than implicit in the IR.  And that has
 been the overall feeling for a few years now.
 

Sorry, I'm afraid I can't agree with you. Instead, I think using barrier to 
describe this kind of dependence is a kind of implicit method. Please note that 
this is not an usual data dependence issue. The stack pointer change doesn't 
have any dependence with memory access at all.

No matter what solution itself is, the problem itself is a quite a common one 
on ISA level, so we should solve it in middle-end, because middle-end is shared 
for all ports. 

My proposal avoids problems in future. Any new ports and new back-end 
implementations needn't explicitly define this hook in a very safe way. But if 
one port wants to unsafely introduce red zone, we can explicitly define this 
hook in back-end. This way, we would avoid the bug in the earliest time. Do you 
really want to hide this problem in back-end silently? And wait others to 
report bug and then waste time to get it fixed again?

The facts I see is over the years, for different ports reported the similar 
issue around this, and somebody tried to fix them. Actually, this kind of 
problem should be fixed in design level. If the first people solve this bug can 
give solution in middle end, we would not need to waste time on filing those 
bugs in bug zilla and fixing them around this problem.

Thanks,
-Jiangning