* Re: [PATCH] Fix PR middle-end/17813
@ 2004-10-18 21:04 Richard Kenner
0 siblings, 0 replies; 23+ messages in thread
From: Richard Kenner @ 2004-10-18 21:04 UTC (permalink / raw)
To: ebotcazou; +Cc: gcc-patches
I obviously missed something. So the debug functions are exposed by
the front-end through a C interface?
No. There's a pragma Export giving that name.
More generally, is a non-Ada-aware debugger sufficient to debug the Ada
part of the front-end?
Pretty much. I don't do work in that code, but the few times I've had to look
at it, I've had no problems. I only very recently started using an Ada-aware
debugger. You just have to know a lot about how things are encoded, but
I'm sure you do!
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-16 21:22 Eric Botcazou
2004-10-17 1:28 ` Roger Sayle
2004-10-17 9:58 ` Matthias Klose
@ 2004-10-18 22:11 ` Eric Botcazou
2 siblings, 0 replies; 23+ messages in thread
From: Eric Botcazou @ 2004-10-18 22:11 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 928 bytes --]
> This is the Ada compiler bootstrap failure on i586 and below, which again
> pertains to stack adjustments. cstand.adb:Create_Standard is miscompiled
> because a stack adjustment is emitted right after a
> __builtin_stack_restore.
Here's the final patch I commited to mainline. Bootstrapped/regtested (all
languages except treelang with upcoming patch for PR middle-end/18045) on
i486-mandrake-linux-gnu, approved offline by Roger.
2004-10-18  Eric Botcazou  <ebotcazou@libertysurf.fr>
Roger Sayle <roger@eyesopen.com>
PR middle-end/17813
* dojump.c (discard_pending_stack_adjust): New function.
(clear_pending_stack_adjust): Call it.
* expr.h (discard_pending_stack_adjust): Declare it.
* explow.c (emit_stack_save): Emit pending stack adjustments
before saving the stack pointer.
(emit_stack_restore): Discard pending stack adjustments before
restoring the stack pointer.
--
Eric Botcazou
[-- Attachment #2: pr17813-2.diff --]
[-- Type: text/x-diff, Size: 2799 bytes --]
Index: dojump.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/dojump.c,v
retrieving revision 1.29
diff -u -p -r1.29 dojump.c
--- dojump.c 8 Sep 2004 07:47:45 -0000 1.29
+++ dojump.c 18 Oct 2004 18:48:23 -0000
@@ -50,6 +50,15 @@ init_pending_stack_adjust (void)
pending_stack_adjust = 0;
}
+/* Discard any pending stack adjustment. This avoid relying on the
+ RTL optimizers to remove useless adjustments when we know the
+ stack pointer value is dead. */
+void discard_pending_stack_adjust (void)
+{
+ stack_pointer_delta -= pending_stack_adjust;
+ pending_stack_adjust = 0;
+}
+
/* When exiting from function, if safe, clear out any pending stack adjust
so the adjustment won't get done.
@@ -64,10 +73,7 @@ clear_pending_stack_adjust (void)
&& EXIT_IGNORE_STACK
&& ! (DECL_INLINE (current_function_decl) && ! flag_no_inline)
&& ! flag_inline_functions)
- {
- stack_pointer_delta -= pending_stack_adjust,
- pending_stack_adjust = 0;
- }
+ discard_pending_stack_adjust ();
}
/* Pop any previously-pushed arguments that have not been popped yet. */
Index: explow.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/explow.c,v
retrieving revision 1.139
diff -u -p -r1.139 explow.c
--- explow.c 8 Sep 2004 08:05:13 -0000 1.139
+++ explow.c 18 Oct 2004 18:48:33 -0000
@@ -965,6 +965,7 @@ emit_stack_save (enum save_level save_le
rtx seq;
start_sequence ();
+ do_pending_stack_adjust ();
/* We must validize inside the sequence, to ensure that any instructions
created by the validize call also get moved to the right place. */
if (sa != 0)
@@ -976,6 +977,7 @@ emit_stack_save (enum save_level save_le
}
else
{
+ do_pending_stack_adjust ();
if (sa != 0)
sa = validize_mem (sa);
emit_insn (fcn (sa, stack_pointer_rtx));
@@ -1032,6 +1034,8 @@ emit_stack_restore (enum save_level save
gen_rtx_MEM (BLKmode, stack_pointer_rtx)));
}
+ discard_pending_stack_adjust ();
+
if (after)
{
rtx seq;
Index: expr.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.h,v
retrieving revision 1.171
diff -u -p -r1.171 expr.h
--- expr.h 8 Sep 2004 18:44:56 -0000 1.171
+++ expr.h 18 Oct 2004 18:48:36 -0000
@@ -499,6 +499,9 @@ extern void expand_var (tree);
arguments waiting to be popped. */
extern void init_pending_stack_adjust (void);
+/* Discard any pending stack adjustment. */
+extern void discard_pending_stack_adjust (void);
+
/* When exiting from function, if safe, clear out any pending stack adjust
so the adjustment won't get done. */
extern void clear_pending_stack_adjust (void);
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-18 20:56 ` Eric Botcazou
@ 2004-10-18 21:17 ` Laurent GUERBY
0 siblings, 0 replies; 23+ messages in thread
From: Laurent GUERBY @ 2004-10-18 21:17 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Richard Kenner
On Mon, 2004-10-18 at 22:39, Eric Botcazou wrote:
> > But that doesn't need an Ada-aware debugger! I use that all the time
> > with non-Ada-aware debuggers. Just do
> >
> > define pn
> > set pn($)
> > end
>
> I obviously missed something. So the debug functions are exposed by the
> front-end through a C interface? More generally, is a non-Ada-aware debugger
> sufficient to debug the Ada part of the front-end?
See ada/treepr.ads and ada/sprint.ads for more documentation, the "pX"
function are pragma Exported so they're callable from gdb even if not
Ada aware.
Laurent
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-18 17:18 Richard Kenner
@ 2004-10-18 20:56 ` Eric Botcazou
2004-10-18 21:17 ` Laurent GUERBY
0 siblings, 1 reply; 23+ messages in thread
From: Eric Botcazou @ 2004-10-18 20:56 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Kenner
> But that doesn't need an Ada-aware debugger! I use that all the time
> with non-Ada-aware debuggers. Just do
>
> define pn
> set pn($)
> end
I obviously missed something. So the debug functions are exposed by the
front-end through a C interface? More generally, is a non-Ada-aware debugger
sufficient to debug the Ada part of the front-end?
--
Eric Botcazou
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
@ 2004-10-18 17:18 Richard Kenner
2004-10-18 20:56 ` Eric Botcazou
0 siblings, 1 reply; 23+ messages in thread
From: Richard Kenner @ 2004-10-18 17:18 UTC (permalink / raw)
To: ebotcazou; +Cc: gcc-patches
Of course, but for debugging a miscompilation of the Ada scanner (e.g.
PR middle-end/18045), having "pn" at hand is almost required if you
are not familiar with the internal structure of the front-end
(e.g. how to walk the node hierarchy) like me.
But that doesn't need an Ada-aware debugger! I use that all the time
with non-Ada-aware debuggers. Just do
define pn
set pn($)
end
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-18 15:44 ` Arnaud Charlet
@ 2004-10-18 16:30 ` Eric Botcazou
0 siblings, 0 replies; 23+ messages in thread
From: Eric Botcazou @ 2004-10-18 16:30 UTC (permalink / raw)
To: gcc-patches; +Cc: Arnaud Charlet, Daniel Jacobowitz
> Note that for doing low-level debugging of Ada code, you can use
> GDB in C mode without too much trouble, so for working on GCC/GNAT itself,
> the fancy Ada mode features are not strictly necessary.
Of course, but for debugging a miscompilation of the Ada scanner (e.g.
PR middle-end/18045), having "pn" at hand is almost required if you are not
familiar with the internal structure of the front-end (e.g. how to walk the
node hierarchy) like me.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-18 14:56 Richard Kenner
@ 2004-10-18 16:03 ` Eric Botcazou
0 siblings, 0 replies; 23+ messages in thread
From: Eric Botcazou @ 2004-10-18 16:03 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Kenner
> Nope. I mean gnatmake blowing up building gnatmem and also in a few of the
> C2 ACATS tests. This is on x86-64.
OK, thanks for the clarification. For some reason, PR tree-opt/17986 shows up
on i686-mingw32, i586-linux, sparc-solaris but apparently not on i686-linux
and x86_64-linux.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-18 15:44 ` Eric Botcazou
@ 2004-10-18 15:44 ` Arnaud Charlet
2004-10-18 16:30 ` Eric Botcazou
0 siblings, 1 reply; 23+ messages in thread
From: Arnaud Charlet @ 2004-10-18 15:44 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Daniel Jacobowitz
> Oops! I really should follow what the GDB folks at ACT are doing... Right,
> it was turned on recently and this is great news. Thanks for the heads up.
Note that for doing low-level debugging of Ada code, you can use
GDB in C mode without too much trouble, so for working on GCC/GNAT itself,
the fancy Ada mode features are not strictly necessary.
That's how we worked on GNAT for years before there we wrote Ada support
in gdb.
Arno
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-17 19:40 ` Daniel Jacobowitz
@ 2004-10-18 15:44 ` Eric Botcazou
2004-10-18 15:44 ` Arnaud Charlet
0 siblings, 1 reply; 23+ messages in thread
From: Eric Botcazou @ 2004-10-18 15:44 UTC (permalink / raw)
To: gcc-patches; +Cc: Daniel Jacobowitz
> Have you tried GDB HEAD lately? I was under the impression that Ada
> support had been turned on (not long ago).
Oops! I really should follow what the GDB folks at ACT are doing... Right,
it was turned on recently and this is great news. Thanks for the heads up.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
@ 2004-10-18 14:56 Richard Kenner
2004-10-18 16:03 ` Eric Botcazou
0 siblings, 1 reply; 23+ messages in thread
From: Richard Kenner @ 2004-10-18 14:56 UTC (permalink / raw)
To: ebotcazou; +Cc: gcc-patches
Do you mean an infinite loop compiling make.adb?
Nope. I mean gnatmake blowing up building gnatmem and also in a few of the C2
ACATS tests. This is on x86-64.
Please assign it to you if you're working on it, this will avoid
parallel work.
And, as I said, I'm not working on it yet: I want to fix the recent ACATS
regressions first and hope that gets fixed in the process.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-17 9:58 ` Matthias Klose
2004-10-17 11:55 ` Eric Botcazou
@ 2004-10-18 12:16 ` Eric Botcazou
1 sibling, 0 replies; 23+ messages in thread
From: Eric Botcazou @ 2004-10-18 12:16 UTC (permalink / raw)
To: gcc-patches; +Cc: Matthias Klose
> with this patch (configured for i486-linux) I get until:
>
> Bootstrap comparison failure!
> ada/a-except.o differs
> ada/ali-util.o differs
> ada/s-crc32.o differs
> make[3]: *** [gnucompare] Error 1
The stage3 s-crc32.o is miscompiled by the stage2 compiler because the stage2
uintp.o is miscompiled by the stage1 compiler because of PR middle-end/18045.
I'm testing a fix for the latter on a full bootstrap (a C+Ada bootstrap
already completed, even without stumbling upon PR tree-opt/17986).
--
Eric Botcazou
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-17 18:48 Richard Kenner
@ 2004-10-18 8:22 ` Eric Botcazou
0 siblings, 0 replies; 23+ messages in thread
From: Eric Botcazou @ 2004-10-18 8:22 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Kenner
> I'm also seeing a miscompilation of gnatmake.
Do you mean an infinite loop compiling make.adb? If so, it's PR
tree-opt/17986. Please assign it to you if you're working on it, this will
avoid parallel work.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-17 17:57 ` Eric Botcazou
@ 2004-10-17 19:40 ` Daniel Jacobowitz
2004-10-18 15:44 ` Eric Botcazou
0 siblings, 1 reply; 23+ messages in thread
From: Daniel Jacobowitz @ 2004-10-17 19:40 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Sun, Oct 17, 2004 at 07:55:33PM +0200, Eric Botcazou wrote:
> > ../src/configure -v --enable-languages=c,c++,java,f95,objc,ada
> > --prefix=/usr --libexecdir=/usr/lib
> > --with-gxx-include-dir=/usr/include/c++/4.0 --enable-shared
> > --with-system-zlib --enable-nls --enable-threads=posix
> > --without-included-gettext --program-suffix=-4.0 --enable-__cxa_atexit
> > --enable-libstdcxx-allocator=mt --enable-clocale=gnu
> > --enable-libstdcxx-debug --enable-java-gc=boehm --enable-java-awt=gtk
> > --enable-mpfr i486-linux
>
> OK, I could reproduce with i486-linux. The bad news is that the stage2 Ada
> front-end appears to be miscompiled by the stage1 compiler and that there is
> currently no Ada-aware debugger that understands location lists...
Have you tried GDB HEAD lately? I was under the impression that Ada
support had been turned on (not long ago).
--
Daniel Jacobowitz
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
@ 2004-10-17 18:48 Richard Kenner
2004-10-18 8:22 ` Eric Botcazou
0 siblings, 1 reply; 23+ messages in thread
From: Richard Kenner @ 2004-10-17 18:48 UTC (permalink / raw)
To: ebotcazou; +Cc: gcc-patches
OK, I could reproduce with i486-linux. The bad news is that the
stage2 Ada front-end appears to be miscompiled by the stage1 compiler
and that there is currently no Ada-aware debugger that understands
location lists...
I'm also seeing a miscompilation of gnatmake.
But there are still ACATS execution failures, so I'm hoping that fixing
those will fix these issues. I hope to get back to that task shortly.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-17 12:03 ` Matthias Klose
@ 2004-10-17 17:57 ` Eric Botcazou
2004-10-17 19:40 ` Daniel Jacobowitz
0 siblings, 1 reply; 23+ messages in thread
From: Eric Botcazou @ 2004-10-17 17:57 UTC (permalink / raw)
To: gcc-patches
> ../src/configure -v --enable-languages=c,c++,java,f95,objc,ada
> --prefix=/usr --libexecdir=/usr/lib
> --with-gxx-include-dir=/usr/include/c++/4.0 --enable-shared
> --with-system-zlib --enable-nls --enable-threads=posix
> --without-included-gettext --program-suffix=-4.0 --enable-__cxa_atexit
> --enable-libstdcxx-allocator=mt --enable-clocale=gnu
> --enable-libstdcxx-debug --enable-java-gc=boehm --enable-java-awt=gtk
> --enable-mpfr i486-linux
OK, I could reproduce with i486-linux. The bad news is that the stage2 Ada
front-end appears to be miscompiled by the stage1 compiler and that there is
currently no Ada-aware debugger that understands location lists...
--
Eric Botcazou
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-17 13:51 ` Eric Botcazou
@ 2004-10-17 17:07 ` Roger Sayle
0 siblings, 0 replies; 23+ messages in thread
From: Roger Sayle @ 2004-10-17 17:07 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Sun, 17 Oct 2004, Eric Botcazou wrote:
> > A slight improvement might be to add a new ignore_pending_stack_adjust
> > function (like clear_pending_stack_adjust) that resets stack_pointer_delta
> > and pending_stack_adjust to zero, and call that in emit_stack_restore.
>
> I have 3 remarks/questions:
> - what about discard_pending_stack_adjust instead? I think that "ignore"
> could fool people into thinking that the stack adjustment is only temporarily
> suspended.
Sure.
> - what about changing clear_pending_stack_adjust into
> maybe_discard_pending_stack_adjust? The rationale is that the typical
> idiom is currently:
>
> clear_pending_stack_adjust ();
> do_pending_stack_adjust ();
>
> which seems odd at best.
I'll agree that the semantics of clear_pending_stack_adjust are a bit
strange, i.e. that this function is occassionally a no-op even if there's
a pending stack adjustment. And it does indeed look like, with the
single exception of expand_call in calls.c, all uses of
clear_pending_stack_adjust are followed by do_pending_stack_adjust, so an
interface/API clean-up (simplification) might be a good thing. Especially
as expand_call's exceptional use looks like a good candidate for the
proposed discard_pending_stack_adjust.
> - I think that stack_pointer_delta should not be reset to zero for strict
> correctness, and instead the same adjustment should be made in the new
> function as in clear_pending_stack_adjust.
Probably. I'll admit that I didn't fully investigate exactly how
stack_pointer_delta needs to be updated in discard_pending_stack_adjust,
just that it needs to be updated (which is why init_pending_stack_adjust
isn't suitable). I'd need to read through more code to convince myself
that a non-zero stack pointer delta is correct/safe for these builtins.
Roger
--
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-17 1:28 ` Roger Sayle
2004-10-17 2:20 ` Jan Hubicka
@ 2004-10-17 13:51 ` Eric Botcazou
2004-10-17 17:07 ` Roger Sayle
1 sibling, 1 reply; 23+ messages in thread
From: Eric Botcazou @ 2004-10-17 13:51 UTC (permalink / raw)
To: gcc-patches; +Cc: Roger Sayle
> This is OK for mainline.
Thanks for the review.
> A slight improvement might be to add a new ignore_pending_stack_adjust
> function (like clear_pending_stack_adjust) that resets stack_pointer_delta
> and pending_stack_adjust to zero, and call that in emit_stack_restore.
I have 3 remarks/questions:
- what about discard_pending_stack_adjust instead? I think that "ignore"
could fool people into thinking that the stack adjustment is only temporarily
suspended.
- what about changing clear_pending_stack_adjust into
maybe_discard_pending_stack_adjust? The rationale is that the typical idiom
is currently:
clear_pending_stack_adjust ();
do_pending_stack_adjust ();
which seems odd at best.
- I think that stack_pointer_delta should not be reset to zero for strict
correctness, and instead the same adjustment should be made in the new
function as in clear_pending_stack_adjust.
> Currently, we'll emit a "dead" stack adjustment immediately prior to
> restoring the stack pointer, and then leave it to the RTL optimizers to
> clean up. All we really need to do is flush the "memory" of the pending
> adjustment.
This was the acknowledged strategy used in the patch. Now, after Honza's
remark, I think yours is better.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-17 11:55 ` Eric Botcazou
@ 2004-10-17 12:03 ` Matthias Klose
2004-10-17 17:57 ` Eric Botcazou
0 siblings, 1 reply; 23+ messages in thread
From: Matthias Klose @ 2004-10-17 12:03 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
Eric Botcazou writes:
> > with this patch (configured for i486-linux) I get until:
> >
> > Bootstrap comparison failure!
> > ada/a-except.o differs
> > ada/ali-util.o differs
> > ada/s-crc32.o differs
> > make[3]: *** [gnucompare] Error 1
>
> Great ;-) Could you post the exact configure line?
bootstrap compiler is gcc-3.3.5.
../src/configure -v --enable-languages=c,c++,java,f95,objc,ada --prefix=/usr --libexecdir=/usr/lib --with-gxx-include-dir=/usr/include/c++/4.0 --enable-shared --with-system-zlib --enable-nls --enable-threads=posix --without-included-gettext --program-suffix=-4.0 --enable-__cxa_atexit --enable-libstdcxx-allocator=mt --enable-clocale=gnu --enable-libstdcxx-debug --enable-java-gc=boehm --enable-java-awt=gtk --enable-mpfr i486-linux
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-17 9:58 ` Matthias Klose
@ 2004-10-17 11:55 ` Eric Botcazou
2004-10-17 12:03 ` Matthias Klose
2004-10-18 12:16 ` Eric Botcazou
1 sibling, 1 reply; 23+ messages in thread
From: Eric Botcazou @ 2004-10-17 11:55 UTC (permalink / raw)
To: gcc-patches; +Cc: Matthias Klose
> with this patch (configured for i486-linux) I get until:
>
> Bootstrap comparison failure!
> ada/a-except.o differs
> ada/ali-util.o differs
> ada/s-crc32.o differs
> make[3]: *** [gnucompare] Error 1
Great ;-) Could you post the exact configure line?
--
Eric Botcazou
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-16 21:22 Eric Botcazou
2004-10-17 1:28 ` Roger Sayle
@ 2004-10-17 9:58 ` Matthias Klose
2004-10-17 11:55 ` Eric Botcazou
2004-10-18 12:16 ` Eric Botcazou
2004-10-18 22:11 ` Eric Botcazou
2 siblings, 2 replies; 23+ messages in thread
From: Matthias Klose @ 2004-10-17 9:58 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
Eric Botcazou writes:
> Hi,
>
> This is the Ada compiler bootstrap failure on i586 and below, which again
> pertains to stack adjustments. cstand.adb:Create_Standard is miscompiled
> because a stack adjustment is emitted right after a __builtin_stack_restore.
with this patch (configured for i486-linux) I get until:
Bootstrap comparison failure!
ada/a-except.o differs
ada/ali-util.o differs
ada/s-crc32.o differs
make[3]: *** [gnucompare] Error 1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-17 1:28 ` Roger Sayle
@ 2004-10-17 2:20 ` Jan Hubicka
2004-10-17 13:51 ` Eric Botcazou
1 sibling, 0 replies; 23+ messages in thread
From: Jan Hubicka @ 2004-10-17 2:20 UTC (permalink / raw)
To: Roger Sayle; +Cc: Eric Botcazou, gcc-patches
>
> On Sat, 16 Oct 2004, Eric Botcazou wrote:
> > 2004-10-16  Eric Botcazou  <ebotcazou@libertysurf.fr>
> >
> > PR middle-end/17813
> > * explow.c (emit_stack_save): Emit pending stack adjustments.
> > (emit_stack_restore): Likewise.
>
>
> This is OK for mainline. A slight improvement might be to add a new
> ignore_pending_stack_adjust function (like clear_pending_stack_adjust)
> that resets stack_pointer_delta and pending_stack_adjust to zero, and
> call that in emit_stack_restore. Currently, we'll emit a "dead" stack
> adjustment immediately prior to restoring the stack pointer, and then
> leave it to the RTL optimizers to clean up. All we really need to do
> is flush the "memory" of the pending adjustment.
RTL optimizers are not terribly smart about optimizing stack
adjustments. WOuld be possible to double check if they are really
elliminated?
>
>
> It's also not clear whether the two builtins __builtin_stack_save and
> __builtin_stack_restore need to be documented in extend.texi, or if
> they're intended purely as internal placeholders. Zdenek?
For purely internal builtins we probably should have something
preventing frontend from recognizing them in the input source so user
can't confuse us.
Honza
>
> Roger
> --
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Fix PR middle-end/17813
2004-10-16 21:22 Eric Botcazou
@ 2004-10-17 1:28 ` Roger Sayle
2004-10-17 2:20 ` Jan Hubicka
2004-10-17 13:51 ` Eric Botcazou
2004-10-17 9:58 ` Matthias Klose
2004-10-18 22:11 ` Eric Botcazou
2 siblings, 2 replies; 23+ messages in thread
From: Roger Sayle @ 2004-10-17 1:28 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=X-UNKNOWN, Size: 929 bytes --]
On Sat, 16 Oct 2004, Eric Botcazou wrote:
> 2004-10-16 Eric Botcazou <ebotcazou@libertysurf.fr>
>
> PR middle-end/17813
> * explow.c (emit_stack_save): Emit pending stack adjustments.
> (emit_stack_restore): Likewise.
This is OK for mainline. A slight improvement might be to add a new
ignore_pending_stack_adjust function (like clear_pending_stack_adjust)
that resets stack_pointer_delta and pending_stack_adjust to zero, and
call that in emit_stack_restore. Currently, we'll emit a "dead" stack
adjustment immediately prior to restoring the stack pointer, and then
leave it to the RTL optimizers to clean up. All we really need to do
is flush the "memory" of the pending adjustment.
It's also not clear whether the two builtins __builtin_stack_save and
__builtin_stack_restore need to be documented in extend.texi, or if
they're intended purely as internal placeholders. Zdenek?
Roger
--
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] Fix PR middle-end/17813
@ 2004-10-16 21:22 Eric Botcazou
2004-10-17 1:28 ` Roger Sayle
` (2 more replies)
0 siblings, 3 replies; 23+ messages in thread
From: Eric Botcazou @ 2004-10-16 21:22 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 652 bytes --]
Hi,
This is the Ada compiler bootstrap failure on i586 and below, which again
pertains to stack adjustments. cstand.adb:Create_Standard is miscompiled
because a stack adjustment is emitted right after a __builtin_stack_restore.
The proposed fix is to emit pending stack adjustments right before
__builtin_stack_save and __builtin_stack_restore. Bootstrapped on
i586-mandrake-linux-gnu (with the workaround for PR tree-opt/17986).
OK for mainline?
2004-10-16  Eric Botcazou  <ebotcazou@libertysurf.fr>
PR middle-end/17813
* explow.c (emit_stack_save): Emit pending stack adjustments.
(emit_stack_restore): Likewise.
--
Eric Botcazou
[-- Attachment #2: pr17813.diff --]
[-- Type: text/x-diff, Size: 1301 bytes --]
Index: explow.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/explow.c,v
retrieving revision 1.139
diff -u -p -r1.139 explow.c
--- explow.c 8 Sep 2004 08:05:13 -0000 1.139
+++ explow.c 16 Oct 2004 16:41:55 -0000
@@ -965,6 +965,7 @@ emit_stack_save (enum save_level save_le
rtx seq;
start_sequence ();
+ do_pending_stack_adjust ();
/* We must validize inside the sequence, to ensure that any instructions
created by the validize call also get moved to the right place. */
if (sa != 0)
@@ -976,6 +977,7 @@ emit_stack_save (enum save_level save_le
}
else
{
+ do_pending_stack_adjust ();
if (sa != 0)
sa = validize_mem (sa);
emit_insn (fcn (sa, stack_pointer_rtx));
@@ -1037,13 +1039,17 @@ emit_stack_restore (enum save_level save
rtx seq;
start_sequence ();
+ do_pending_stack_adjust ();
emit_insn (fcn (stack_pointer_rtx, sa));
seq = get_insns ();
end_sequence ();
emit_insn_after (seq, after);
}
else
- emit_insn (fcn (stack_pointer_rtx, sa));
+ {
+ do_pending_stack_adjust ();
+ emit_insn (fcn (stack_pointer_rtx, sa));
+ }
}
/* Invoke emit_stack_save on the nonlocal_goto_save_area for the current
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2004-10-18 22:07 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-10-18 21:04 [PATCH] Fix PR middle-end/17813 Richard Kenner
-- strict thread matches above, loose matches on Subject: below --
2004-10-18 17:18 Richard Kenner
2004-10-18 20:56 ` Eric Botcazou
2004-10-18 21:17 ` Laurent GUERBY
2004-10-18 14:56 Richard Kenner
2004-10-18 16:03 ` Eric Botcazou
2004-10-17 18:48 Richard Kenner
2004-10-18 8:22 ` Eric Botcazou
2004-10-16 21:22 Eric Botcazou
2004-10-17 1:28 ` Roger Sayle
2004-10-17 2:20 ` Jan Hubicka
2004-10-17 13:51 ` Eric Botcazou
2004-10-17 17:07 ` Roger Sayle
2004-10-17 9:58 ` Matthias Klose
2004-10-17 11:55 ` Eric Botcazou
2004-10-17 12:03 ` Matthias Klose
2004-10-17 17:57 ` Eric Botcazou
2004-10-17 19:40 ` Daniel Jacobowitz
2004-10-18 15:44 ` Eric Botcazou
2004-10-18 15:44 ` Arnaud Charlet
2004-10-18 16:30 ` Eric Botcazou
2004-10-18 12:16 ` Eric Botcazou
2004-10-18 22:11 ` Eric Botcazou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).