RE: [RFC] Add middle end hook for stack red zone size

RE: [RFC] Add middle end hook for stack red zone size

[Jiangning Liu]

Hi,

One month ago, I sent out this RFC to *gcc-patches* mail list, but I didn't receive any response yet. So I'm forwarding this mail to *gcc* mail list. Can anybody here really give feedback to me?

Appreciate your help in advance!

-Jiangning

-----Original Message-----
From: Ramana Radhakrishnan [mailto:ramana.radhakrishnan@linaro.org] 
Sent: Tuesday, July 19, 2011 6:18 PM
To: Jiangning Liu
Cc: gcc-patches@gcc.gnu.org; vmakarov@redhat.com; dje.gcc@gmail.com; Richard Henderson; Ramana Radhakrishnan
Subject: Re: [RFC] Add middle end hook for stack red zone size

2011/7/19 Jiangning Liu <jiangning.liu@arm.com>:
>
> I see a lot of feedbacks on other posts, but mine is still with ZERO
> response in the past 3 weeks, so I'm wondering if I made any mistake in my
> process? Who can help me?

It would be worth CC'ing the other relevant target maintainers as well
to get some feedback since the patch touches ARM, x86 and Powerpc.
I've added the maintainers for i386 and PPC to the CC list using the
email addresses from the MAINTAINERS file.

Thanks,
Ramana

>
> Thanks,
> -Jiangning
>
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org]
> On Behalf Of Jiangning Liu
> Sent: Tuesday, July 05, 2011 8:32 AM
> To: gcc-patches@gcc.gnu.org; rguenth@gcc.gnu.org
> Subject: RE: [RFC] Add middle end hook for stack red zone size
>
> PING...
>
> I just merged with the latest code base and generated new patch as attached.
>
> Thanks,
> -Jiangning
>
>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>> owner@gcc.gnu.org] On Behalf Of Jiangning Liu
>> Sent: 2011年6月28日 4:38 PM
>> To: gcc-patches@gcc.gnu.org
>> Subject: [RFC] Add middle end hook for stack red zone size
>>
>> This patch is to fix PR38644, which is a bug with long history about
>> stack red zone access, and PR30282 is correlated.
>>
>> Originally red zone concept is not exposed to middle-end, and back-end
>> uses special logic to add extra memory barrier RTL and help the
>> correct dependence in middle-end. This way different back-ends must
>> handle red zone problem by themselves. For example, X86 target
>> introduced function
>> ix86_using_red_zone() to judge red zone access, while POWER introduced
>> offset_below_red_zone_p() to judge it. Note that they have different
>> semantics, but the logic in caller sites of back-end uses them to
>> decide whether adding memory barrier RTL or not. If back-end
>> incorrectly handles this, bug would be introduced.
>>
>> Therefore, the correct method should be middle-end handles red zone
>> related things to avoid the burden in different back-ends. To be
>> specific for PR38644, this middle-end problem causes incorrect
>> behavior for ARM target.
>> This patch exposes red zone concept to middle-end by introducing a
>> middle-end/back-end hook TARGET_STACK_RED_ZONE_SIZE defined in
>> target.def, and by default its value is 0. Back-end may redefine this
>> function to provide concrete red zone size according to specific ABI
>> requirements.
>>
>> In middle end, scheduling dependence is modified by using this hook
>> plus checking stack frame pointer adjustment instruction to decide
>> whether memory references need to be all flushed out or not. In
>> theory, if TARGET_STACK_RED_ZONE_SIZE is defined correctly, back-end
>> would not be required to specially handle this scheduling dependence
>> issue by introducing extra memory barrier RTL.
>>
>> In back-end, the following changes are made to define the hook,
>> 1) For X86, TARGET_STACK_RED_ZONE_SIZE is redefined to be
>> ix86_stack_red_zone_size() in i386.c, which is an newly introduced
>> function.
>> 2) For POWER, TARGET_STACK_RED_ZONE_SIZE is redefined to be
>> rs6000_stack_red_zone_size() in rs6000.c, which is also a newly
>> defined function.
>> 3) For ARM and others, TARGET_STACK_RED_ZONE_SIZE is defined to be
>> default_stack_red_zone_size in targhooks.c, and this function returns
>> 0, which means ARM eabi and others don't support red zone access at all.
>>
>> In summary, the relationship between ABI and red zone access is like
>> below,
>>
>> -----------------------------------------------------------------
>> |   ARCH       |  ARM  |       X86     |    POWER      | others |
>> |--------------|-------|---------------|---------------|--------|
>> |    ABI       | EABI  | MS_64 | other |   AIX  |  V4  |        |
>> |--------------|-------|-------|-------|--------|------|--------|
>> | RED ZONE     |  No   |  YES  |  No   |  YES   |  No  |   No   |
>> |--------------|-------|-------|-------|--------|------|--------|
>> | RED ZONE SIZE|   0   |  128  |   0   |220/288 |   0  |    0   |
>> -----------------------------------------------------------------
>>
>> Thanks,
>> -Jiangning
>
>
>
>

RE: [RFC] Add middle end hook for stack red zone size

[Joern Rennecke]

[-- Attachment #1: Type: text/plain, Size: 2462 bytes --]

Quoting Jiangning Liu <jiangning.liu@arm.com>:

> Hi,
>
> One month ago, I sent out this RFC to *gcc-patches* mail list, but I  
>  didn't receive any response yet. So I'm forwarding this mail to   
> *gcc* mail list. Can anybody here really give feedback to me?

Well, I couldn't approve any patch, but I can point out some issues
with your patch.

First, it's missing a ChangeLog, and you don't state how you have tested it.
And regarding the code in sched_analyze_1, I think you'll get false
positives with alloca, and false negatives when registers are involved
to compute offsets or to restore the stack pointer from.

FWIW, I think generally blunt scheduling barriers should be avoided, and
instead the dependencies made visible to the scheduler.
E.g., I've been working with another architecture with a redzone, where at
-fno-omit-frame-pointer, the prologue can put pretend_args into the redzone,
then after stack adjustment and frame allocation, these arguments are
accessed via the frame pointer.

With the attached patchlet, alias analysis works for this situation too, so
no blunt scheduling block is required.

Likewise, with stack adjustments, they should not affect scheduling in
general, but be considered to clobber the part of the frame that is
being exposed to interrupt writes either before or after the adjustment.
At the moment, each port that wants to have such selective scheduling
blockages has to define a stack_adjust pattern with a memory clobber  
in a parallel, with a memref that shows the high water mark of possible
interrupt stack writes.
Prima facia it would seem convenient if you only had to tell the middle-end
about the redzone size, and it could figure out the implicit clobbers when
the stack is changed.  However, when a big stack adjustment is being made,
or the stack is realigned, or restored from the frame pointer / another
register where it was saved due to realignment, the adjustment is not
so obvious.  I'm not sure if you can actually create an robust interface
that's simpler to use than putting the right memory clobber in the stack
adjust pattern.  Note also that the redzone size can vary from function
to function depending on ABI-altering attributes, in particular for interrupt
functions, which can also have different incoming and outgoing redzone
sizes.  Plus, you can have an NMI / reset handler which can use the stack
like an ordinary address register.

[-- Attachment #2: tmp --]
[-- Type: text/plain, Size: 1159 bytes --]

2009-09-23  Joern Rennecke <joern.rennecke@embecosm.com>

	* alias.c (base_alias_check): Allow for aliases between stack-
	and frame-pointer referenced memory.

Index: alias.c
===================================================================
--- alias.c	(revision 1646)
+++ alias.c	(revision 1647)
@@ -1751,6 +1751,17 @@ base_alias_check (rtx x, rtx y, enum mac
   if (GET_CODE (x_base) != ADDRESS && GET_CODE (y_base) != ADDRESS)
     return 0;
 
+  /* If both are stack references, one via the stack pointer, the other via
+     the frame pointer, there can be an alias.
+     E.g.: gcc.c-torture/execute/20041113-1.c -O3 -g  */
+  if (GET_CODE (x_base) == ADDRESS
+      && (XEXP (x_base, 0) == stack_pointer_rtx
+	  || XEXP (x_base, 0) == hard_frame_pointer_rtx)
+      && GET_CODE (y_base) == ADDRESS
+      && (XEXP (y_base, 0) == stack_pointer_rtx
+	  || XEXP (y_base, 0) == hard_frame_pointer_rtx))
+    return 1;
+
   /* If one address is a stack reference there can be no alias:
      stack references using different base registers do not alias,
      a stack reference can not alias a parameter, and a stack reference

RE: [RFC] Add middle end hook for stack red zone size

[Jiangning Liu]

Joern,

Thanks for your valuable feedback! This is only a RFC, and I will send out formal patch along with ChangLog later on. 

Basically, my patch is only to add new dependence in scheduler, and it only blocks some instruction movements, so it is NO RISK to compiler correctness. For whatever stack pointer changes you gave in different scenarios, the current code base should already work. My patch intends neither to replace old dependences, nor maximize the scheduler capability due to the existence of red zone in stack. It is only to block the memory access moving over stack pointer adjustment if distance is beyond red zone size, which is an OS requirement due to interruption existence. 

Stack adjustment in epilogue is a very general usage in stack frame. It's quite necessary to solve the general problem in middle-end rather than in back-end. Also, that old patch you attached is to solve the data dependence between two memory accesses, but stack pointer doesn't really have data dependence with memory access without using stack pointer, so they have different stories. Alternative solution of without adding blunt scheduling barrier is we insert an independent pass before scheduler to create RTL barrier by using the same interface stack_red_zone_size, but it would really be an over-design, if we add a new pass only for this *small* functionality.

In my patch, *abs* of offset is being used, so you are right that it's possible to get false positive to be too conservative, but there won't exist false negative, because my code would only add new dependences. 

Since the compilation is based on function, it would be OK if red zone size varies due to different ABI. Could you please tell me exactly on what system being supported by GCC red zone size can be different for incoming and outgoing? And also how scheduler guarantee the correctness in current code base? Anyway, I don't think my patch will break the original solution.

Thanks,
-Jiangning

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org]
> On Behalf Of Joern Rennecke
> Sent: Tuesday, July 26, 2011 10:33 AM
> To: Jiangning Liu
> Cc: gcc@gcc.gnu.org; gcc-patches@gcc.gnu.org; vmakarov@redhat.com;
> dje.gcc@gmail.com; Richard Henderson; Ramana Radhakrishnan; 'Ramana
> Radhakrishnan'
> Subject: RE: [RFC] Add middle end hook for stack red zone size
> 
> Quoting Jiangning Liu <jiangning.liu@arm.com>:
> 
> > Hi,
> >
> > One month ago, I sent out this RFC to *gcc-patches* mail list, but I
> >  didn't receive any response yet. So I'm forwarding this mail to
> > *gcc* mail list. Can anybody here really give feedback to me?
> 
> Well, I couldn't approve any patch, but I can point out some issues with your patch.
> 
> First, it's missing a ChangeLog, and you don't state how you have tested it.
> And regarding the code in sched_analyze_1, I think you'll get false positives with
> alloca, and false negatives when registers are involved to compute offsets or to
> restore the stack pointer from.
> 
> FWIW, I think generally blunt scheduling barriers should be avoided, and instead
> the dependencies made visible to the scheduler.
> E.g., I've been working with another architecture with a redzone, where at -fno-
> omit-frame-pointer, the prologue can put pretend_args into the redzone, then after
> stack adjustment and frame allocation, these arguments are accessed via the frame
> pointer.
> 
> With the attached patchlet, alias analysis works for this situation too, so no blunt
> scheduling block is required.
> 
> Likewise, with stack adjustments, they should not affect scheduling in general, but
> be considered to clobber the part of the frame that is being exposed to interrupt
> writes either before or after the adjustment.
> At the moment, each port that wants to have such selective scheduling blockages
> has to define a stack_adjust pattern with a memory clobber in a parallel, with a
> memref that shows the high water mark of possible interrupt stack writes.
> Prima facia it would seem convenient if you only had to tell the middle-end about
> the redzone size, and it could figure out the implicit clobbers when the stack is
> changed.  However, when a big stack adjustment is being made, or the stack is
> realigned, or restored from the frame pointer / another register where it was
> saved due to realignment, the adjustment is not so obvious.  I'm not sure if you can
> actually create an robust interface that's simpler to use than putting the right
> memory clobber in the stack adjust pattern.  Note also that the redzone size can
> vary from function to function depending on ABI-altering attributes, in particular
> for interrupt functions, which can also have different incoming and outgoing
> redzone sizes.  Plus, you can have an NMI / reset handler which can use the stack
> like an ordinary address register.

Re: [RFC] Add middle end hook for stack red zone size

[Jakub Jelinek]

On Mon, Aug 01, 2011 at 11:44:04AM +0800, Jiangning Liu wrote:
> It's quite necessary to solve the general problem in middle-end rather than in back-end.

That's what we disagree on.  All back-ends but ARM are able to handle it
right, why can't ARM too?  The ABI rules for stack handling in the epilogues
are simply too diverse and complex to be handled easily in the scheduler.

	Jakub

Re: [RFC] Add middle end hook for stack red zone size

[Joseph S. Myers]

On Mon, 1 Aug 2011, Jakub Jelinek wrote:

> On Mon, Aug 01, 2011 at 11:44:04AM +0800, Jiangning Liu wrote:
> > It's quite necessary to solve the general problem in middle-end rather than in back-end.
> 
> That's what we disagree on.  All back-ends but ARM are able to handle it
> right, why can't ARM too?  The ABI rules for stack handling in the epilogues
> are simply too diverse and complex to be handled easily in the scheduler.

Given that the long-standing open bugs relating to scheduling and stack 
adjustments (30282, 38644) include issues for Power that do not yet appear 
to have been fixed, even if other back ends are able to handle it right 
they don't seem to do so at present.

-- 
Joseph S. Myers
joseph@codesourcery.com

RE: [RFC] Add middle end hook for stack red zone size

[Jiangning Liu]

The answer is ARM can. However, if you look into the bugs PR30282 and 
PR38644, PR44199, you may find in history, there are several different cases

in different ports reporting the similar failures, covering x86, PowerPC and

ARM. You are right, they were all fixed in back-ends in the past, but we
should 
fix the bug in a general way to make GCC infrastructure stronger, rather 
than fixing the problem target-by-target and case-by-case! If you further 
look into the back-end fixes in x86 and PowerPC, you may find they looks 
quite similar in back-ends. 

Thanks,
-Jiangning

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org]
> On Behalf Of Jakub Jelinek
> Sent: Monday, August 01, 2011 5:12 PM
> To: Jiangning Liu
> Cc: 'Joern Rennecke'; gcc@gcc.gnu.org; gcc-patches@gcc.gnu.org;
> vmakarov@redhat.com; dje.gcc@gmail.com; Richard Henderson; Ramana
> Radhakrishnan; 'Ramana Radhakrishnan'
> Subject: Re: [RFC] Add middle end hook for stack red zone size
> 
> On Mon, Aug 01, 2011 at 11:44:04AM +0800, Jiangning Liu wrote:
> > It's quite necessary to solve the general problem in middle-end rather
than in
> back-end.
> 
> That's what we disagree on.  All back-ends but ARM are able to handle it
> right, why can't ARM too?  The ABI rules for stack handling in the
epilogues
> are simply too diverse and complex to be handled easily in the scheduler.
> 
> 	Jakub

Re: [RFC] Add middle end hook for stack red zone size

[Jakub Jelinek]

On Mon, Aug 01, 2011 at 06:14:27PM +0800, Jiangning Liu wrote:
> ARM. You are right, they were all fixed in back-ends in the past, but we
> should 
> fix the bug in a general way to make GCC infrastructure stronger, rather 
> than fixing the problem target-by-target and case-by-case! If you further 
> look into the back-end fixes in x86 and PowerPC, you may find they looks 
> quite similar in back-ends. 
> 

Red zone is only one difficulty, your patch is e.g. completely ignoring
existence of biased stack pointers (e.g. SPARC -m64 has them).
Some targets have stack growing in opposite direction, etc.
We have really a huge amount of very diverse ABIs and making the middle-end
grok what is an invalid stack access is difficult.

	Jakub

RE: [RFC] Add middle end hook for stack red zone size

[Jiangning Liu]

Hi Jakub,

Appreciate for your valuable comments!

I think SPARC V9 ABI doesn't have red zone defined, right? So
stack_red_zone_size should be defined as zero by default, the scheduler
would block moving memory accesses across stack adjustment no matter what
the offset is. I don't see any risk here. Also, in my patch function *abs*
is being used to avoid the opposite stack direction issue as you mentioned.

Some people like you insist on the ABI diversity, and actually I agree with
you on this. But part of the ABI definition is general for all targets. The
point here is memory access beyond stack red zone should be avoided, which
is the general part of ABI that compiler should guarantee. For this general
part, middle end should take the responsibility.

Thanks,
-Jiangning

> -----Original Message-----
> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: Monday, August 01, 2011 6:31 PM
> To: Jiangning Liu
> Cc: 'Joern Rennecke'; gcc@gcc.gnu.org; gcc-patches@gcc.gnu.org;
> vmakarov@redhat.com; dje.gcc@gmail.com; Richard Henderson; Ramana
> Radhakrishnan; 'Ramana Radhakrishnan'
> Subject: Re: [RFC] Add middle end hook for stack red zone size
> 
> On Mon, Aug 01, 2011 at 06:14:27PM +0800, Jiangning Liu wrote:
> > ARM. You are right, they were all fixed in back-ends in the past, but
> we
> > should
> > fix the bug in a general way to make GCC infrastructure stronger,
> rather
> > than fixing the problem target-by-target and case-by-case! If you
> further
> > look into the back-end fixes in x86 and PowerPC, you may find they
> looks
> > quite similar in back-ends.
> >
> 
> Red zone is only one difficulty, your patch is e.g. completely ignoring
> existence of biased stack pointers (e.g. SPARC -m64 has them).
> Some targets have stack growing in opposite direction, etc.
> We have really a huge amount of very diverse ABIs and making the
> middle-end
> grok what is an invalid stack access is difficult.
> 
> 	Jakub

Re: [RFC] Add middle end hook for stack red zone size

[Richard Earnshaw]

On 01/08/11 10:11, Jakub Jelinek wrote:
> On Mon, Aug 01, 2011 at 11:44:04AM +0800, Jiangning Liu wrote:
>> It's quite necessary to solve the general problem in middle-end rather than in back-end.
> 
> That's what we disagree on.  All back-ends but ARM are able to handle it
> right, why can't ARM too?  The ABI rules for stack handling in the epilogues
> are simply too diverse and complex to be handled easily in the scheduler.

Because the vast majority of back-ends (ie those without red zones)
shouldn't have to deal with this mess.  This is something the MI code
should be able to work out and deal with itself.  Then the compiler will
at least generate safe code, rather than randomly moving things about
and allowing potentially unsafe writes/reads from beyond the allocated
stack region.

We should build the compiler defensively, but then allow for more
aggressive optimizations to disable the defences when the back-end wants
to take on the responsibility.  Not the other way around.

R.