Discussion:
[PATCH] Fix stack red zone bug (PR38644)
Jiangning Liu
2011-09-26 10:26:05 UTC
Permalink
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
+++ b/gcc/doc/tm.texi.in
@@ -11223,6 +11223,22 @@ to the stack. Therefore, this hook should return
true in general, but
false for naked functions. The default implementation always returns true.
@end deftypefn

+@hook TARGET_STACK_USING_RED_ZONE
+This hook returns true if the target has a red zone (an area beyond the
+current extent of the stack that cannot be modified by asynchronous events
+on the processor).
+
+If this hook returns false then the compiler mid-end will not move an
access
+to memory in the stack frame past a stack adjustment insn.
+
+If this hook returns true then the compiler mid-end will assume that it is
+safe to move an access to memory in the stack frame past a stack adjustment
+insn. The target back-end must emit scheduling barrier insns when this is
+unsafe.
+
+The default is to return false which is safe and appropriate for most
targets.
+@end deftypefn
+
@hook TARGET_CONST_ANCHOR
On some architectures it can take multiple instructions to synthesize
a constant. If there is another constant already in a register that
diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
index ed592c8..575b20c 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -2332,6 +2332,14 @@ sched_analyze_1 (struct deps_desc *deps, rtx x, rtx
insn)
FIRST_STACK_REG);
}
#endif
+ /* If the stack pointer is being modified, flush out all memory
+ references as they may become invalid if moved across the
+ stack adjustment. */
+ if (!targetm.calls.stack_using_red_zone ()
+ && (dest == stack_pointer_rtx))
+ {
+ flush_pending_lists (deps, insn, true, true);
+ }
}
else if (MEM_P (dest))
{
diff --git a/gcc/target.def b/gcc/target.def
index 1e09ba7..5e2cd38 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -2110,6 +2110,13 @@ DEFHOOK
bool, (void),
hook_bool_void_true)

+/* Return true if target uses stack red zone. */
+DEFHOOK
+(stack_using_red_zone,
+ "",
+ bool, (void),
+ hook_bool_void_false)
+
/* Return an rtx for the static chain for FNDECL. If INCOMING_P is true,
then it should be for the callee; otherwise for the caller. */
DEFHOOK
diff --git a/gcc/testsuite/gcc.target/arm/stack-red-zone.c
b/gcc/testsuite/gcc.target/arm/stack-red-zone.c
new file mode 100644
index 0000000..97f7aae
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/stack-red-zone.c
@@ -0,0 +1,12 @@
+/* No stack red zone. PR38644. */
+/* { dg-options "-mthumb -O2" } */
+/* { dg-final { scan-assembler "ldrb\[^\n\]*\\n\[\t \]*add\[\t
\]*sp\[^\n\]*\\n\[\t \]*@\[^\n\]*\\n\[\t \]*pop" } } */
+
+extern int doStreamReadBlock (int *, char *, int size, int);
+
+char readStream (int *s)
+{
+ char c = 0;
+ doStreamReadBlock (s, &c, 1, *s);
+ return c;
+}
Andrew Pinski
2011-09-26 21:30:38 UTC
Permalink
On Mon, Sep 26, 2011 at 3:26 AM, Jiangning Liu <***@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
Jiangning Liu
2011-09-27 02:28:41 UTC
Permalink
> -----Original Message-----
> From: Andrew Pinski [mailto:***@gmail.com]
> Sent: Tuesday, September 27, 2011 5:31 AM
> To: Jiangning Liu
> Cc: gcc-***@gcc.gnu.org
> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>
> On Mon, Sep 26, 2011 at 3:26 AM, Jiangning Liu <***@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
Andrew Pinski
2011-09-27 02:44:12 UTC
Permalink
On Mon, Sep 26, 2011 at 7:28 PM, Jiangning Liu <***@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
Jiangning Liu
2011-09-27 03:32:07 UTC
Permalink
> 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
Richard Guenther
2011-09-27 07:41:00 UTC
Permalink
On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu <***@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
>
>
>
>
>
>
>
>
Jiangning Liu
2011-09-28 01:49:57 UTC
Permalink
> -----Original Message-----
> From: Richard Guenther [mailto:***@gmail.com]
> Sent: Tuesday, September 27, 2011 3:41 PM
> To: Jiangning Liu
> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>
> On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu <***@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
> >
> >
> >
> >
> >
> >
> >
> >
Richard Guenther
2011-09-28 08:38:51 UTC
Permalink
On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu <***@arm.com> wrote:
>
>
>> -----Original Message-----
>> From: Richard Guenther [mailto:***@gmail.com]
>> Sent: Tuesday, September 27, 2011 3:41 PM
>> To: Jiangning Liu
>> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
>> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>>
>> On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu <***@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
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>
>
>
>
>
Jiangning Liu
2011-09-28 09:10:31 UTC
Permalink
> -----Original Message-----
> From: Richard Guenther [mailto:***@gmail.com]
> Sent: Wednesday, September 28, 2011 4:39 PM
> To: Jiangning Liu
> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>
> On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu <***@arm.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:***@gmail.com]
> >> Sent: Tuesday, September 27, 2011 3:41 PM
> >> To: Jiangning Liu
> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> >>
> >> On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
> <***@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, 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
> >> >
> >> >
> >> >
> >> >
> >> >
> >> >
> >> >
> >> >
> >
> >
> >
> >
> >
Richard Guenther
2011-09-28 09:19:54 UTC
Permalink
On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu <***@arm.com> wrote:
>
>
>> -----Original Message-----
>> From: Richard Guenther [mailto:***@gmail.com]
>> Sent: Wednesday, September 28, 2011 4:39 PM
>> To: Jiangning Liu
>> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
>> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>>
>> On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu <***@arm.com>
>> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: Richard Guenther [mailto:***@gmail.com]
>> >> Sent: Tuesday, September 27, 2011 3:41 PM
>> >> To: Jiangning Liu
>> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
>> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>> >>
>> >> On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
>> <***@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 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
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
>> >
>> >
>> >
>> >
>> >
>
>
>
>
>
Jiangning Liu
2011-09-28 09:40:42 UTC
Permalink
> -----Original Message-----
> From: Richard Guenther [mailto:***@gmail.com]
> Sent: Wednesday, September 28, 2011 5:20 PM
> To: Jiangning Liu
> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>
> On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu <***@arm.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:***@gmail.com]
> >> Sent: Wednesday, September 28, 2011 4:39 PM
> >> To: Jiangning Liu
> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> >>
> >> On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu
> <***@arm.com>
> >> wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: Richard Guenther [mailto:***@gmail.com]
> >> >> Sent: Tuesday, September 27, 2011 3:41 PM
> >> >> To: Jiangning Liu
> >> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> >> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> >> >>
> >> >> On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
> >> <***@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 it is a *biggest* issue. It is only one of an issue. But for
such a simple issue, we had a bunch of bug reports already over the years.
So many people took so much time on this. Sometimes a very simple bug may be
very annoying from engineering point of view. Right?

> 3) I'm sure there are very
> many
> latent backend bugs not related to red-zone
>

Of course I'm talking about the bugs related to stack red zone. I mean for
other back-ends other than x86/powerpc/ARM, there might still exit bug
around stack 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.
>

Well, for GCC I think the concept of middle end is pretty much like the
concept of "shared code". Even some passes can work on both GIMPLE and RTL.
I mean this specific stack red zone issue can be abstracted in shared for
all ports, so it is quite related to middle end. I didn't say it has nothing
to do with back-end. I'm arguing middle-end can help the simplified
implementation of back-end around this. As I mentioned middle-end wouldn't
live itself without specific target support on back-end, right?

> > 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, 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
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >
> >> >
> >> >
> >> >
> >> >
> >
> >
> >
> >
> >
Richard Guenther
2011-09-28 09:55:40 UTC
Permalink
On Wed, Sep 28, 2011 at 11:40 AM, Jiangning Liu <***@arm.com> wrote:
>
>
>> -----Original Message-----
>> From: Richard Guenther [mailto:***@gmail.com]
>> Sent: Wednesday, September 28, 2011 5:20 PM
>> To: Jiangning Liu
>> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
>> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>>
>> On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu <***@arm.com>
>> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: Richard Guenther [mailto:***@gmail.com]
>> >> Sent: Wednesday, September 28, 2011 4:39 PM
>> >> To: Jiangning Liu
>> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
>> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>> >>
>> >> On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu
>> <***@arm.com>
>> >> wrote:
>> >> >
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Richard Guenther [mailto:***@gmail.com]
>> >> >> Sent: Tuesday, September 27, 2011 3:41 PM
>> >> >> To: Jiangning Liu
>> >> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
>> >> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>> >> >>
>> >> >> On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
>> >> <***@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 barriers to avoid scheduling
certain instructions. That other targets don't need this because they
have a red-zone doesn't mean you have to "handle a red-zone" - you
clearly don't - you simply need to properly model dependences for
the scheduler.

Richard.
Jiangning Liu
2011-09-29 03:13:10 UTC
Permalink
> -----Original Message-----
> From: Richard Guenther [mailto:***@gmail.com]
> Sent: Wednesday, September 28, 2011 5:56 PM
> To: Jiangning Liu
> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>
> On Wed, Sep 28, 2011 at 11:40 AM, Jiangning Liu <***@arm.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:***@gmail.com]
> >> Sent: Wednesday, September 28, 2011 5:20 PM
> >> To: Jiangning Liu
> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> >>
> >> On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu
> <***@arm.com>
> >> wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: Richard Guenther [mailto:***@gmail.com]
> >> >> Sent: Wednesday, September 28, 2011 4:39 PM
> >> >> To: Jiangning Liu
> >> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> >> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> >> >>
> >> >> On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu
> >> <***@arm.com>
> >> >> wrote:
> >> >> >
> >> >> >
> >> >> >> -----Original Message-----
> >> >> >> From: Richard Guenther [mailto:***@gmail.com]
> >> >> >> Sent: Tuesday, September 27, 2011 3:41 PM
> >> >> >> To: Jiangning Liu
> >> >> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> >> >> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> >> >> >>
> >> >> >> On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
> >> >> <***@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 barriers to avoid
> scheduling
> certain instructions. That other targets don't need this because they
> have a red-zone doesn't mean you have to "handle a red-zone" - you
> clearly don't - you simply need to properly model dependences for
> the scheduler.
>

Richard,

My understanding here "model dependences" is explicitly adding barrier in
back-end for stack red zone purpose. And this is also how x86 and power pc
are doing in back-end. So it definitely means we have to "handle a
red-zone". Otherwise, would not see so many codes in back-ends explicitly
marking with "red zone" comments.

GCC has the following 41 ports right now,

alpha
arc
arm
avr
bfin
cris
crx
fr30
frv
h8300
i386
ia64
iq2000
lm32
m32c
m32r
m68hc11
m68k
mcore
mep
microblaze
mips
mmix
mn10300
moxie
pa
pdp11
picochip
rs6000
rx
s390
score
sh
soft-fp
sparc
spu
stormy16
v850
vax
vms
xtensa

And I only see X86_MS_64 ABI and POWER_AIX ABI are supporting red zone.

-------------------------------------------------------------
| ARCH | ARM | X86 | POWER | others |
|----------|-------|---------------|---------------|--------|
| ABI | EABI | MS_64 | other | AIX | V4 | |
|----------|-------|-------|-------|--------|------|--------|
| RED ZONE | No | YES | No | YES | No | No |
-------------------------------------------------------------

So are you really expecting all targets other than x86_MS_64 and POWER_AIX
have to explicitly write code in back-end to insert barrier just because of
not supporting stack red zone? This is ridiculous for me.

For all other targets potentially they all may have bugs around this. (Maybe
somebody working for other targets can help to have a try on the cases given
in PR38644 and PR30282.) I have to say scheduler is unsafe at this point.

Hopefully you can consider this problem again.

Thanks,
-Jiangning



> Richard.
Richard Guenther
2011-09-29 09:02:57 UTC
Permalink
On Thu, Sep 29, 2011 at 5:13 AM, Jiangning Liu <***@arm.com> wrote:
>
>
>> -----Original Message-----
>> From: Richard Guenther [mailto:***@gmail.com]
>> Sent: Wednesday, September 28, 2011 5:56 PM
>> To: Jiangning Liu
>> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
>> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>>
>> On Wed, Sep 28, 2011 at 11:40 AM, Jiangning Liu <***@arm.com>
>> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: Richard Guenther [mailto:***@gmail.com]
>> >> Sent: Wednesday, September 28, 2011 5:20 PM
>> >> To: Jiangning Liu
>> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
>> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>> >>
>> >> On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu
>> <***@arm.com>
>> >> wrote:
>> >> >
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Richard Guenther [mailto:***@gmail.com]
>> >> >> Sent: Wednesday, September 28, 2011 4:39 PM
>> >> >> To: Jiangning Liu
>> >> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
>> >> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>> >> >>
>> >> >> On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu
>> >> <***@arm.com>
>> >> >> wrote:
>> >> >> >
>> >> >> >
>> >> >> >> -----Original Message-----
>> >> >> >> From: Richard Guenther [mailto:***@gmail.com]
>> >> >> >> Sent: Tuesday, September 27, 2011 3:41 PM
>> >> >> >> To: Jiangning Liu
>> >> >> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
>> >> >> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>> >> >> >>
>> >> >> >> On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
>> >> >> <***@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 barriers to avoid
>> scheduling
>> certain instructions.  That other targets don't need this because they
>> have a red-zone doesn't mean you have to "handle a red-zone" - you
>> clearly don't - you simply need to properly model dependences for
>> the scheduler.
>>
>
> Richard,
>
> My understanding here "model dependences" is explicitly adding barrier in
> back-end for stack red zone purpose. And this is also how x86 and power pc
> are doing in back-end. So it definitely means we have to "handle a
> red-zone". Otherwise, would not see so many codes in back-ends explicitly
> marking with "red zone" comments.
>
> GCC has the following 41 ports right now,
>
>  alpha
>  arc
>  arm
>  avr
>  bfin
>  cris
>  crx
>  fr30
>  frv
>  h8300
>  i386
>  ia64
>  iq2000
>  lm32
>  m32c
>  m32r
>  m68hc11
>  m68k
>  mcore
>  mep
>  microblaze
>  mips
>  mmix
>  mn10300
>  moxie
>  pa
>  pdp11
>  picochip
>  rs6000
>  rx
>  s390
>  score
>  sh
>  soft-fp
>  sparc
>  spu
>  stormy16
>  v850
>  vax
>  vms
>  xtensa
>
> And I only see X86_MS_64 ABI and POWER_AIX ABI are supporting red zone.
>
> -------------------------------------------------------------
> |   ARCH   |  ARM  |       X86     |    POWER      | others |
> |----------|-------|---------------|---------------|--------|
> |    ABI   | EABI  | MS_64 | other |   AIX  |  V4  |        |
> |----------|-------|-------|-------|--------|------|--------|
> | RED ZONE |  No   |  YES  |  No   |  YES   |  No  |   No   |
> -------------------------------------------------------------
>
> So are you really expecting all targets other than x86_MS_64 and POWER_AIX
> have to explicitly write code in back-end to insert barrier just because of
> not supporting stack red zone? This is ridiculous for me.
>
> For all other targets potentially they all may have bugs around this. (Maybe
> somebody working for other targets can help to have a try on the cases given
> in PR38644 and PR30282.) I have to say scheduler is unsafe at this point.
>
> Hopefully you can consider this problem again.

As far as I see only targets know where to best insert the barrier. After
all the targets emit the prologue/epilogue code, not the middle-end.

But well, I'm just arguing from the point that we want explicit representations
of dependences (and other things) rather than implicit ones where then
middle-end code needs to remember to check some weird target macros
(same argument applies - why should somebody writing a middle-end
pass need to worry about target specialities with dependences - why
can't the targets "work ok by default"?)

I don't know the scheduler or prologue/epilogue code very well (nor the
problem at hand).

Richard.

> Thanks,
> -Jiangning
>
>
>
>> Richard.
>
>
>
>
>
Jiangning Liu
2011-09-29 10:08:50 UTC
Permalink
> -----Original Message-----
> From: Richard Guenther [mailto:***@gmail.com]
> Sent: Thursday, September 29, 2011 5:03 PM
> To: Jiangning Liu
> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>
> On Thu, Sep 29, 2011 at 5:13 AM, Jiangning Liu <***@arm.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:***@gmail.com]
> >> Sent: Wednesday, September 28, 2011 5:56 PM
> >> To: Jiangning Liu
> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> >>
> >> On Wed, Sep 28, 2011 at 11:40 AM, Jiangning Liu
> <***@arm.com>
> >> wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: Richard Guenther [mailto:***@gmail.com]
> >> >> Sent: Wednesday, September 28, 2011 5:20 PM
> >> >> To: Jiangning Liu
> >> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> >> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> >> >>
> >> >> On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu
> >> <***@arm.com>
> >> >> wrote:
> >> >> >
> >> >> >
> >> >> >> -----Original Message-----
> >> >> >> From: Richard Guenther [mailto:***@gmail.com]
> >> >> >> Sent: Wednesday, September 28, 2011 4:39 PM
> >> >> >> To: Jiangning Liu
> >> >> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> >> >> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> >> >> >>
> >> >> >> On Wed, Sep 28, 2011 at 3:49 AM, Jiangning Liu
> >> >> <***@arm.com>
> >> >> >> wrote:
> >> >> >> >
> >> >> >> >
> >> >> >> >> -----Original Message-----
> >> >> >> >> From: Richard Guenther [mailto:***@gmail.com]
> >> >> >> >> Sent: Tuesday, September 27, 2011 3:41 PM
> >> >> >> >> To: Jiangning Liu
> >> >> >> >> Cc: Andrew Pinski; gcc-***@gcc.gnu.org
> >> >> >> >> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> >> >> >> >>
> >> >> >> >> On Tue, Sep 27, 2011 at 5:32 AM, Jiangning Liu
> >> >> >> <***@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 barriers to avoid
> >> scheduling
> >> certain instructions.  That other targets don't need this because
> they
> >> have a red-zone doesn't mean you have to "handle a red-zone" - you
> >> clearly don't - you simply need to properly model dependences for
> >> the scheduler.
> >>
> >
> > Richard,
> >
> > My understanding here "model dependences" is explicitly adding
> barrier in
> > back-end for stack red zone purpose. And this is also how x86 and
> power pc
> > are doing in back-end. So it definitely means we have to "handle a
> > red-zone". Otherwise, would not see so many codes in back-ends
> explicitly
> > marking with "red zone" comments.
> >
> > GCC has the following 41 ports right now,
> >
> >  alpha
> >  arc
> >  arm
> >  avr
> >  bfin
> >  cris
> >  crx
> >  fr30
> >  frv
> >  h8300
> >  i386
> >  ia64
> >  iq2000
> >  lm32
> >  m32c
> >  m32r
> >  m68hc11
> >  m68k
> >  mcore
> >  mep
> >  microblaze
> >  mips
> >  mmix
> >  mn10300
> >  moxie
> >  pa
> >  pdp11
> >  picochip
> >  rs6000
> >  rx
> >  s390
> >  score
> >  sh
> >  soft-fp
> >  sparc
> >  spu
> >  stormy16
> >  v850
> >  vax
> >  vms
> >  xtensa
> >
> > And I only see X86_MS_64 ABI and POWER_AIX ABI are supporting red
> zone.
> >
> > -------------------------------------------------------------
> > |   ARCH   |  ARM  |       X86     |    POWER      | others |
> > |----------|-------|---------------|---------------|--------|
> > |    ABI   | EABI  | MS_64 | other |   AIX  |  V4  |        |
> > |----------|-------|-------|-------|--------|------|--------|
> > | RED ZONE |  No   |  YES  |  No   |  YES   |  No  |   No   |
> > -------------------------------------------------------------
> >
> > So are you really expecting all targets other than x86_MS_64 and
> POWER_AIX
> > have to explicitly write code in back-end to insert barrier just
> because of
> > not supporting stack red zone? This is ridiculous for me.
> >
> > For all other targets potentially they all may have bugs around this.
> (Maybe
> > somebody working for other targets can help to have a try on the
> cases given
> > in PR38644 and PR30282.) I have to say scheduler is unsafe at this
> point.
> >
> > Hopefully you can consider this problem again.
>
> As far as I see only targets know where to best insert the barrier.

Yes. Agree. So this is why I say it would a burden for back-end if we use
barrier to handle this case.

> After
> all the targets emit the prologue/epilogue code, not the middle-end.
>

I can see some prologue/epilogue hooks, but they are only for the top level
hook-ups.

> But well, I'm just arguing from the point that we want explicit
> representations
> of dependences (and other things) rather than implicit ones where then
> middle-end code needs to remember to check some weird target macros
> (same argument applies - why should somebody writing a middle-end
> pass need to worry about target specialities with dependences - why
> can't the targets "work ok by default"?)

My answer is the world is not perfect, and we need to hide something for
middle end. This is why so many TARGET HOOKs are being defined in target.def
and used in middle end. This way middle end doesn't need to care what
back-end implements the details at all. I think a lot of middle-end codes
already checked "wired" target macros. I think what you are concerning is
scheduler shouldn't check this "wired" target macros. IMHO, I also think
scheduler should be pure enough and not ask it to be contaminated by this
implicit barriers. But the problem here is we are trying to build a robust
GCC infrastructure rather than only a robust scheduler. Without the
diversity of so many back-ends, the world would be pure as you want.

>
> I don't know the scheduler or prologue/epilogue code very well (nor the
> problem at hand).
>

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.

> Richard.
>
> > Thanks,
> > -Jiangning
> >
> >
> >
> >> Richard.
> >
> >
> >
> >
> >
Jakub Jelinek
2011-09-29 10:13:32 UTC
Permalink
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
Jiangning Liu
2011-09-30 01:13:28 UTC
Permalink
> -----Original Message-----
> From: Jakub Jelinek [mailto:***@redhat.com]
> Sent: Thursday, September 29, 2011 6:14 PM
> To: Jiangning Liu
> Cc: 'Richard Guenther'; Andrew Pinski; gcc-***@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
Richard Sandiford
2011-09-30 08:15:03 UTC
Permalink
"Jiangning Liu" <***@arm.com> writes:
>> -----Original Message-----
>> From: Jakub Jelinek [mailto:***@redhat.com]
>> Sent: Thursday, September 29, 2011 6:14 PM
>> To: Jiangning Liu
>> Cc: 'Richard Guenther'; Andrew Pinski; gcc-***@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
Jiangning Liu
2011-09-30 11:54:05 UTC
Permalink
> -----Original Message-----
> From: Richard Sandiford [mailto:***@googlemail.com]
> Sent: Friday, September 30, 2011 4:15 PM
> To: Jiangning Liu
> Cc: 'Jakub Jelinek'; 'Richard Guenther'; Andrew Pinski; gcc-
> ***@gcc.gnu.org
> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>
> "Jiangning Liu" <***@arm.com> writes:
> >> -----Original Message-----
> >> From: Jakub Jelinek [mailto:***@redhat.com]
> >> Sent: Thursday, September 29, 2011 6:14 PM
> >> To: Jiangning Liu
> >> Cc: 'Richard Guenther'; Andrew Pinski; gcc-***@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, but they are all in
middle end. This is the logic I'm seeing in GCC. 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.

You can fix MIPS, so how would the remaining targets be going? Do we need to
write document to warn all other targets on this somewhere? If we do this,
why don't we use hook-up code itself to document it?

Thanks,
-Jiangning

> Richard
Richard Sandiford
2011-09-30 12:46:03 UTC
Permalink
"Jiangning Liu" <***@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
Richard Guenther
2011-09-30 12:56:42 UTC
Permalink
On Fri, Sep 30, 2011 at 2:46 PM, Richard Sandiford
<***@linaro.org> wrote:
> "Jiangning Liu" <***@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
>
Jiangning Liu
2011-10-09 07:54:09 UTC
Permalink
> -----Original Message-----
> From: Richard Guenther [mailto:***@gmail.com]
> Sent: Friday, September 30, 2011 8:57 PM
> To: Jiangning Liu; Jakub Jelinek; Richard Guenther; Andrew Pinski; gcc-
> ***@gcc.gnu.org; ***@linaro.org
> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>
> On Fri, Sep 30, 2011 at 2:46 PM, Richard Sandiford
> <***@linaro.org> wrote:
> > "Jiangning Liu" <***@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 G.,

Yeah, this sounds like a good justification.

Could you please explain more about how RTL PRE (gcse.c) can avoid hoisting
ld/st over intrinsic alloca?

If RTL PRE doesn't handle it at all, then I would say it is a potential bug.

If RTL PRE does handle it, is the current interface in this pass checking
the memory barrier? And how about other passes you mentioned?

Thanks,
-Jiangning

> Richard.
>
> > Richard
> >
Richard Sandiford
2011-09-30 12:52:26 UTC
Permalink
Richard Sandiford <***@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
Jiangning Liu
2011-10-09 09:33:33 UTC
Permalink
> -----Original Message-----
> From: Richard Sandiford <***@linaro.org>
> Date: Fri, Sep 30, 2011 at 8:46 PM
> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> To: Jiangning Liu <***@arm.com>
> Cc: Jakub Jelinek <***@redhat.com>, Richard Guenther
> <***@gmail.com>, Andrew Pinski <***@gmail.com>,
> gcc-***@gcc.gnu.org
>
>
> "Jiangning Liu" <***@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 to middle-end for some scenarios other than scheduler. For
example,

1) Other current RTL passes are also using memory barrier.
2) We may add more new RTL passes in future.

So many middle-end passes would have to take care of both memory barrier and
this new hook simultaneously.

==============

Let me compare those two solutions again with table below,

-----------------------------------------------------------------
| | Fix in middle-end | Fix in back-end |
| | with new hook | with memory barrier |
|------------|-----------------------|--------------------------|
|Advantage | One fix for | Middle-end passes use |
| | all back-ends. | barrier as an unique |
| | | interface. |
|------------|-----------------------|--------------------------|
|Disadvantage| One more interface. | Fix different ports |
| | New burden to many | one by one. Burden in |
| | middle-end passes | back-ends. New ports |
| | (even new passes). | have to know this. |
-----------------------------------------------------------------

I have to say this justification sounds reasonable to me so far. Under this
situation, I don’t have strong preference any longer.

Let me consider it again. Appreciate your explanation for me with great
patience on this headache problem!

-Jiangning

>
> 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
Richard Henderson
2011-09-30 19:05:01 UTC
Permalink
On 09/29/2011 06:13 PM, Jiangning Liu wrote:
>
>
>> -----Original Message-----
>> From: Jakub Jelinek [mailto:***@redhat.com]
>> Sent: Thursday, September 29, 2011 6:14 PM
>> To: Jiangning Liu
>> Cc: 'Richard Guenther'; Andrew Pinski; gcc-***@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>" "r<i>,l<i>")))
(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~
Jiangning Liu
2011-10-09 08:11:52 UTC
Permalink
> -----Original Message-----
> From: Richard Henderson [mailto:***@redhat.com]
> Sent: Saturday, October 01, 2011 3:05 AM
> To: Jiangning Liu
> Cc: 'Jakub Jelinek'; 'Richard Guenther'; Andrew Pinski; gcc-
> ***@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:***@redhat.com]
> >> Sent: Thursday, September 29, 2011 6:14 PM
> >> To: Jiangning Liu
> >> Cc: 'Richard Guenther'; Andrew Pinski; gcc-***@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>" "r<i>,l<i>")))
> (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~
Steven Bosscher
2011-10-02 15:24:24 UTC
Permalink
On Wed, Sep 28, 2011 at 11:10 AM, Jiangning Liu <***@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
Richard Sandiford
2011-09-29 09:22:14 UTC
Permalink
Andrew Pinski <***@gmail.com> writes:
> On Mon, Sep 26, 2011 at 7:28 PM, Jiangning Liu <***@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.

<paranoia>Did you mean Richard E?</paranoia>

Richard
Jiangning Liu
2011-09-27 02:36:38 UTC
Permalink
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 is
+safe to move an access to memory in the stack frame past a stack adjustment
+insn. The target back-end must emit scheduling barrier insns when this is
+unsafe.
+
+The default is to return false which is safe and appropriate for most
targets.
+@end deftypefn
+
@hook TARGET_CONST_ANCHOR
On some architectures it can take multiple instructions to synthesize
a constant. If there is another constant already in a register that
diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
index ed592c8..575b20c 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -2332,6 +2332,14 @@ sched_analyze_1 (struct deps_desc *deps, rtx x, rtx
insn)
FIRST_STACK_REG);
}
#endif
+ /* If the stack pointer is being modified, flush out all memory
+ references as they may become invalid if moved across the
+ stack adjustment. */
+ if (!targetm.calls.stack_using_red_zone ()
+ && (dest == stack_pointer_rtx))
+ {
+ flush_pending_lists (deps, insn, true, true);
+ }
}
else if (MEM_P (dest))
{
diff --git a/gcc/target.def b/gcc/target.def
index 1e09ba7..5e2cd38 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -2110,6 +2110,13 @@ DEFHOOK
bool, (void),
hook_bool_void_true)

+/* Return true if target uses stack red zone. */
+DEFHOOK
+(stack_using_red_zone,
+ "",
+ bool, (void),
+ hook_bool_void_false)
+
/* Return an rtx for the static chain for FNDECL. If INCOMING_P is true,
then it should be for the callee; otherwise for the caller. */
DEFHOOK
diff --git a/gcc/testsuite/gcc.target/arm/stack-red-zone.c
b/gcc/testsuite/gcc.target/arm/stack-red-zone.c
new file mode 100644
index 0000000..4888707
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/stack-red-zone.c
@@ -0,0 +1,13 @@
+/* No stack red zone. PR38644. */
+/* { dg-options "-mthumb -O2" } */
+/* { dg-final { scan-assembler "ldrb\[^\n\]*\\n\[\t \]*add\[\t
\]*sp\[^\n\]*\\n\[\t \]*@\[^\n\]*\\n\[\t \]*pop" } } */
+/* { dg-require-effective-target arm_thumb1_ok } */
+
+extern int doStreamReadBlock (int *, char *, int size, int);
+
+char readStream (int *s)
+{
+ char c = 0;
+ doStreamReadBlock (s, &c, 1, *s);
+ return c;
+}
Uros Bizjak
2011-09-27 07:19:37 UTC
Permalink
On Tue, Sep 27, 2011 at 4:36 AM, Jiangning Liu <***@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.
Jiangning Liu
2011-09-28 06:23:32 UTC
Permalink
> > -static inline bool
> > +extern bool
>
> static bool
>
> >  ix86_using_red_zone (void)
> >  {
> >   return TARGET_RED_ZONE && !TARGET_64BIT_MS_ABI;
> > @@ -35958,6 +35958,9 @@ ix86_autovectorize_vector_sizes (void)
> >  #define TARGET_STACK_PROTECT_FAIL ix86_stack_protect_fail
> >  #endif
> >

...

> >
> > +/* Return true if the ABI allows red zone access.  */
> > +extern bool
>
> static bool
>
> > +rs6000_stack_using_red_zone (void)
> > +{
> > +   return (DEFAULT_ABI != ABI_V4);
> > +}
> > +
>

Uros,

Thanks for your review. Accept and new patch is as below. No change for
ChangeLog.

Thanks,
-Jiangning

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


/* Return true if a red-zone is in use. */

-static inline bool
+static bool
ix86_using_red_zone (void)
{
return TARGET_RED_ZONE && !TARGET_64BIT_MS_ABI;
@@ -35958,6 +35958,9 @@ ix86_autovectorize_vector_sizes (void)
#define TARGET_STACK_PROTECT_FAIL ix86_stack_protect_fail
#endif

+#undef TARGET_STACK_USING_RED_ZONE
+#define TARGET_STACK_USING_RED_ZONE ix86_using_red_zone
+
#undef TARGET_FUNCTION_VALUE
#define TARGET_FUNCTION_VALUE ix86_function_value

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 1ab57e5..1e64d14 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -1537,6 +1537,9 @@ static const struct attribute_spec
rs6000_attribute_table[] =
#define TARGET_STACK_PROTECT_FAIL rs6000_stack_protect_fail
#endif

+#undef TARGET_STACK_USING_RED_ZONE
+#define TARGET_STACK_USING_RED_ZONE rs6000_stack_using_red_zone
+
/* MPC604EUM 3.5.2 Weak Consistency between Multiple Processors
The PowerPC architecture requires only weak consistency among
processors--that is, memory accesses between processors need not be
@@ -20660,6 +20663,13 @@ rs6000_restore_saved_cr (rtx reg, int
using_mfcr_multiple)
}
}

+/* Return true if the ABI allows red zone access. */
+static bool
+rs6000_stack_using_red_zone (void)
+{
+ return (DEFAULT_ABI != ABI_V4);
+}
+
/* Return true if OFFSET from stack pointer can be clobbered by signals.
V.4 doesn't have any stack cushion, AIX ABIs have 220 or 288 bytes
below stack pointer not cloberred by signals. */
@@ -20667,7 +20677,7 @@ rs6000_restore_saved_cr (rtx reg, int
using_mfcr_multiple)
static inline bool
offset_below_red_zone_p (HOST_WIDE_INT offset)
{
- return offset < (DEFAULT_ABI == ABI_V4
+ return offset < (!TARGET_STACK_USING_RED_ZONE
? 0
: TARGET_32BIT ? -220 : -288);
}
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 335c1d1..c848806 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -11332,6 +11332,22 @@ to the stack. Therefore, this hook should return
true in general, but
false for naked functions. The default implementation always returns true.
@end deftypefn

+@deftypefn {Target Hook} bool TARGET_STACK_USING_RED_ZONE (void)
+This hook returns true if the target has a red zone (an area beyond the
+current extent of the stack that cannot be modified by asynchronous events
+on the processor).
+
+If this hook returns false then the compiler mid-end will not move an
access
+to memory in the stack frame past a stack adjustment insn.
+
+If this hook returns true then the compiler mid-end will assume that it is
+safe to move an access to memory in the stack frame past a stack adjustment
+insn. The target back-end must emit scheduling barrier insns when this is
+unsafe.
+
+The default is to return false which is safe and appropriate for most
targets.
+@end deftypefn
+
@deftypevr {Target Hook} {unsigned HOST_WIDE_INT} TARGET_CONST_ANCHOR
On some architectures it can take multiple instructions to synthesize
a constant. If there is another constant already in a register that
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 6783826..0fa9e10 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -11223,6 +11223,22 @@ to the stack. Therefore, this hook should return
true in general, but
false for naked functions. The default implementation always returns true.
@end deftypefn

+@hook TARGET_STACK_USING_RED_ZONE
+This hook returns true if the target has a red zone (an area beyond the
+current extent of the stack that cannot be modified by asynchronous events
+on the processor).
+
+If this hook returns false then the compiler mid-end will not move an
access
+to memory in the stack frame past a stack adjustment insn.
+
+If this hook returns true then the compiler mid-end will assume that it is
+safe to move an access to memory in the stack frame past a stack adjustment
+insn. The target back-end must emit scheduling barrier insns when this is
+unsafe.
+
+The default is to return false which is safe and appropriate for most
targets.
+@end deftypefn
+
@hook TARGET_CONST_ANCHOR
On some architectures it can take multiple instructions to synthesize
a constant. If there is another constant already in a register that
diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
index ed592c8..575b20c 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -2332,6 +2332,14 @@ sched_analyze_1 (struct deps_desc *deps, rtx x, rtx
insn)
FIRST_STACK_REG);
}
#endif
+ /* If the stack pointer is being modified, flush out all memory
+ references as they may become invalid if moved across the
+ stack adjustment. */
+ if (!targetm.calls.stack_using_red_zone ()
+ && (dest == stack_pointer_rtx))
+ {
+ flush_pending_lists (deps, insn, true, true);
+ }
}
else if (MEM_P (dest))
{
diff --git a/gcc/target.def b/gcc/target.def
index 1e09ba7..5e2cd38 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -2110,6 +2110,13 @@ DEFHOOK
bool, (void),
hook_bool_void_true)

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