Patch ping

Patch ping

[Paolo Bonzini]

http://gcc.gnu.org/ml/gcc-patches/2004-04/msg00384.html
Enables -fweb at -O2 and -frename-registers at -O1 if variable
tracking is available for the target's default debugging information
format.

Ok for mainline?

Paolo

Re: Patch ping

[Geoff Keating]

"Paolo Bonzini" <bonzini@gnu.org> writes:

> http://gcc.gnu.org/ml/gcc-patches/2004-04/msg00384.html
> Enables -fweb at -O2 and -frename-registers at -O1 if variable
> tracking is available for the target's default debugging information
> format.
> 
> Ok for mainline?

This is OK, assuming you bootstrapped it on at least one platform.

-- 
- Geoffrey Keating <geoffk@geoffk.org>

Re: Patch ping

[Andreas Jaeger]

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

Geoff Keating <geoffk@geoffk.org> writes:

> "Paolo Bonzini" <bonzini@gnu.org> writes:
>
>> http://gcc.gnu.org/ml/gcc-patches/2004-04/msg00384.html
>> Enables -fweb at -O2 and -frename-registers at -O1 if variable
>> tracking is available for the target's default debugging information
>> format.
>> 
>> Ok for mainline?
>
> This is OK, assuming you bootstrapped it on at least one platform.

Paolo,

on which platform did you bootstrap this?  It causes build errors on
both Linux/x86-64 and Linux/ia64 as reported by Andreas Schwab and
myself on the main GCC list yesterday.

Please fix the regressions that your patch has caused.

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: Patch ping

[Paolo Bonzini]

> Paolo,
>
> on which platform did you bootstrap this?

i686-pc-linux-gnu, all languages except Ada/treelang.

> It causes build errors on
> both Linux/x86-64 and Linux/ia64 as reported by Andreas Schwab and
> myself on the main GCC list yesterday.
>
> Please fix the regressions that your patch has caused.

As I wrote on gcc, I don't think I can be blamed on this.  The bugs are not
in the code I touched, but only in the code I enabled.  All I can do is
disabling -frename-registers on the affected archs, as per the attached
patch disable-ada-rename-regs.patch.

Also, I guess would have been found earlier if the Ada testsuite had tested
all optimization levels instead of -O2 only (and it used to be -O0, which
seems really strange to me!).  Can you please try the attached
ada-testsuite.patch on a two-three days old gcc, on Linux/x86-64 and/or
Linux/ia64?

Ok to commit both patches?

Paolo


begin 666 ada-testsuite.patch
M26YD97@Z(')U;E]A8V%T<PH]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]
M/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]"E)#4R!F
M:6QE.B O8W9S+V=C8R]G8V,O9V-C+W1E<W1S=6ET92]A9&$O86-A=',O<G5N
M7V%C871S+'8*<F5T<FEE=FEN9R!R979I<VEO;B Q+C4*9&EF9B M=2 M<C$N
M-2!R=6Y?86-A=',*+2TM(')U;E]A8V%T<PDX($IA;B R,# T(#$U.C$Y.C,V
M("TP,# P"3$N-0HK*RL@<G5N7V%C871S"3$Y($%P<B R,# T(# Y.C,T.C$V
M("TP,# P"D! ("TU,2PT("LU,2PX($! "B *(&-H;6]D("MX(&AO<W1?9VYA
M=&UA:V4*( HM97AE8R D=&5S=&1I<B]R=6Y?86QL+G-H("(D0"(**V=C8V9L
M86=S/2U/," D=&5S=&1I<B]R=6Y?86QL+G-H("(D0"(**V=C8V9L86=S/2U/
M,2 D=&5S=&1I<B]R=6Y?86QL+G-H("(D0"(**V=C8V9L86=S/2U/,B D=&5S
M=&1I<B]R=6Y?86QL+G-H("(D0"(**V=C8V9L86=S/2U/,R D=&5S=&1I<B]R
M=6Y?86QL+G-H("(D0"(**V=C8V9L86=S/2U/<R D=&5S=&1I<B]R=6Y?86QL
M+G-H("(D0"(*26YD97@Z(')U;E]A;&PN<V@*/3T]/3T]/3T]/3T]/3T]/3T]
M/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]
M/3T]/0I20U,@9FEL93H@+V-V<R]G8V,O9V-C+V=C8R]T97-T<W5I=&4O861A
M+V%C871S+W)U;E]A;&PN<V@L=@IR971R:65V:6YG(')E=FES:6]N(#$N,34*
M9&EF9B M=2 M<C$N,34@<G5N7V%L;"YS: HM+2T@<G5N7V%L;"YS: DQ-R!!
M<'(@,C P-" Q-SHQ-#HQ." M,# P, DQ+C$U"BLK*R!R=6Y?86QL+G-H"3$Y
M($%P<B R,# T(# Y.C,T.C$V("TP,# P"D! ("TY+#@@*SDL."! 0 H@(R!G
M8V-F;&%G<STB+4\S("UF;VUI="UF<F%M92UP;VEN=&5R("UF=6YR;VQL+6%L
M;"UL;V]P<R M9FEN;&EN92UF=6YC=&EO;G,B"B C(&=N871F;&%G<STB+6=N
M871.(@H@"BUG8V-F;&%G<STB+4\R(@HM9VYA=&9L86=S/2(M9VYA='=S(@HK
M.B D>V=C8V9L86=S/2(M3S(B?0HK.B D>V=N871F;&%G<STB+6=N871W<R)]
M"B *('1A<F=E=%]R=6X@*"D@>PH@)"H*0$ @+3<X+#8@*S<X+#D@0$ *(&1I
M<W!L87D@8'1Y<&4@9VYA=&UA:V5@"B!G;F%T;',@+78@/CX@)&1I<B]A8V%T
M<RYL;V<*(&1I<W!L87D@(B(**V1I<W!L87D@9V-C(&]P=&EO;G,@87)E("(D
M>V=C8V9L86=S?2(**V1I<W!L87D@9VYA=&UA:V4@;W!T:6]N<R!A<F4@(B1[
M9V-C9FQA9W-]("1[9VYA=&9L86=S?2(**V1I<W!L87D@(B(*( H@9&ES<&QA
M>2 B"0D]/3T@86-A=',@<W5P<&]R=" ]/3TB"B!D:7-P;&%Y7VYO96]L(")'
M96YE<F%T:6YG('-U<'!O<G0@9FEL97,N+BXB"D! ("TQ-3(L-B K,34U+#D@
M0$ *(&1I<W!L87D@(B!D;VYE+B(*(&1I<W!L87D@(B(*(&1I<W!L87D@(@D)
M/3T](&%C871S('1E<W1S(#T]/2(**V1I<W!L87D@(B(**V1I<W!L87D@9V-C
M(&]P=&EO;G,@87)E("(D>V=C8V9L86=S?2(**V1I<W!L87D@9VYA=&UA:V4@
M;W!T:6]N<R!A<F4@(B1[9V-C9FQA9W-]("1[9VYA=&9L86=S?2(*( H@:68@
M6R D(R M97$@,"!=.R!T:&5N"B @("!C:&%P=&5R<SU@8V0@)&1I<B]T97-T
0<SL@96-H;R!;82UZ72I@"@``
`
end

begin 666 disable-ada-rename-regs.patch
M26YD97@Z(&DS.#8O="UL:6YU>#8T"CT]/3T]/3T]/3T]/3T]/3T]/3T]/3T]
M/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T*
M4D-3(&9I;&4Z("]C=G,O9V-C+V=C8R]G8V,O8V]N9FEG+VDS.#8O="UL:6YU
M>#8T+'8*<F5T<FEE=FEN9R!R979I<VEO;B Q+C4*9&EF9B M=2 M<C$N-2!T
M+6QI;G5X-C0*+2TM(&DS.#8O="UL:6YU>#8T"3(X($YO=B R,# R(#$T.C0W
M.C R("TP,# P"3$N-0HK*RL@:3,X-B]T+6QI;G5X-C0),3D@07!R(#(P,#0@
M,#DZ-#,Z-#$@+3 P,# *0$ @+3$X+#,@*S$X+#4@0$ *(",@8F5C875S92!T
M:&5N(%]?1E)!345?14Y$7U\@;6EG:'0@;F]T(&)E('1H92!L87-T('1H:6YG
M(&EN("YE:%]F<F%M90H@(R!S96-T:6]N+@H@0U)44U151D9?5%]#1DQ!1U,@
M/2 M9FYO+6]M:70M9G)A;64M<&]I;G1E<B M9FYO+6%S>6YC:')O;F]U<RUU
M;G=I;F0M=&%B;&5S"BL**U1?041!7T-&3$%'4R ]("UF;F\M<F5N86UE+7)E
M9VES=&5R<PI);F1E>#H@:6$V-"]T+6EA-C0*/3T]/3T]/3T]/3T]/3T]/3T]
M/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]
M/3T]/0I20U,@9FEL93H@+V-V<R]G8V,O9V-C+V=C8R]C;VYF:6<O:6$V-"]T
M+6EA-C0L=@IR971R:65V:6YG(')E=FES:6]N(#$N,C$*9&EF9B M=2 M<C$N
M,C$@="UI838T"BTM+2!I838T+W0M:6$V- DR.2!/8W0@,C P,R Q-CHR,3HS
M-B M,# P, DQ+C(Q"BLK*R!I838T+W0M:6$V- DQ.2!!<'(@,C P-" P.3HT
M,SHT,2 M,# P, I 0" M-#DL,R K-#DL-2! 0 H@"B C(&=E;F%T=')T86(@
M9V5N97)A=&5S('9E<GD@;&]N9R!S=')I;F<@;&ET97)A;',N"B!I;G-N+6%T
M=')T86(N;RUW87)N(#T@+5=N;RUE<G)O<@HK"BM47T%$05]#1DQ!1U,@/2 M
59FYO+7)E;F%M92UR96=I<W1E<G,*
`
end

Re: Patch ping

[Arnaud Charlet]

> Ok to commit both patches?

Could you resubmit your patch in plain text, and/or as standard
mime attachments ? thanks.

uuencoded format is kind of antiquated these days, and not easily
dealt without painful manual processing.

Arno

Re: Patch ping

[Paolo Bonzini]

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

> uuencoded format is kind of antiquated these days, and not easily
> dealt without painful manual processing.

Dense MS Outlook Express. :-(

Paolo

[-- Attachment #2: ada-testsuite.patch --]
[-- Type: application/octet-stream, Size: 1646 bytes --]

Index: run_acats
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/ada/acats/run_acats,v
retrieving revision 1.5
diff -u -r1.5 run_acats
--- run_acats	8 Jan 2004 15:19:36 -0000	1.5
+++ run_acats	19 Apr 2004 09:34:16 -0000
@@ -51,4 +51,8 @@
 
 chmod +x host_gnatmake
 
-exec $testdir/run_all.sh "$@"
+gccflags=-O0 $testdir/run_all.sh "$@"
+gccflags=-O1 $testdir/run_all.sh "$@"
+gccflags=-O2 $testdir/run_all.sh "$@"
+gccflags=-O3 $testdir/run_all.sh "$@"
+gccflags=-Os $testdir/run_all.sh "$@"
Index: run_all.sh
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/ada/acats/run_all.sh,v
retrieving revision 1.15
diff -u -r1.15 run_all.sh
--- run_all.sh	17 Apr 2004 17:14:18 -0000	1.15
+++ run_all.sh	19 Apr 2004 09:34:16 -0000
@@ -9,8 +9,8 @@
 # gccflags="-O3 -fomit-frame-pointer -funroll-all-loops -finline-functions"
 # gnatflags="-gnatN"
 
-gccflags="-O2"
-gnatflags="-gnatws"
+: ${gccflags="-O2"}
+: ${gnatflags="-gnatws"}
 
 target_run () {
 $*
@@ -78,6 +78,9 @@
 display `type gnatmake`
 gnatls -v >> $dir/acats.log
 display ""
+display gcc options are "${gccflags}"
+display gnatmake options are "${gccflags} ${gnatflags}"
+display ""
 
 display "		=== acats support ==="
 display_noeol "Generating support files..."
@@ -152,6 +155,9 @@
 display " done."
 display ""
 display "		=== acats tests ==="
+display ""
+display gcc options are "${gccflags}"
+display gnatmake options are "${gccflags} ${gnatflags}"
 
 if [ $# -eq 0 ]; then
    chapters=`cd $dir/tests; echo [a-z]*`

[-- Attachment #3: disable-ada-rename-regs.patch --]
[-- Type: application/octet-stream, Size: 947 bytes --]

Index: i386/t-linux64
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/i386/t-linux64,v
retrieving revision 1.5
diff -u -r1.5 t-linux64
--- i386/t-linux64	28 Nov 2002 14:47:02 -0000	1.5
+++ i386/t-linux64	19 Apr 2004 09:43:41 -0000
@@ -18,3 +18,5 @@
 # because then __FRAME_END__ might not be the last thing in .eh_frame
 # section.
 CRTSTUFF_T_CFLAGS = -fno-omit-frame-pointer -fno-asynchronous-unwind-tables
+
+T_ADA_CFLAGS = -fno-rename-registers
Index: ia64/t-ia64
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/ia64/t-ia64,v
retrieving revision 1.21
diff -u -r1.21 t-ia64
--- ia64/t-ia64	29 Oct 2003 16:21:36 -0000	1.21
+++ ia64/t-ia64	19 Apr 2004 09:43:41 -0000
@@ -49,3 +49,5 @@
 
 # genattrtab generates very long string literals.
 insn-attrtab.o-warn = -Wno-error
+
+T_ADA_CFLAGS = -fno-rename-registers

Re: Patch ping

[Arnaud Charlet]

> Dense MS Outlook Express. :-(

You first patch causes cpu usage problem as mentioned by Laurent,
so that's probably not reasonable. In addition, maintaining a
baseline of all failures on all these levels of switches will also be
very painful.

-O0 was clearly the simplest (in particular on platforms where optimizations
are somewhat fragile), and has the advantage of being clean on a few
platforms, making it easier to perform comparisons.

-O2 is a good compromise, since as shown recently, it is often the
case that middle-end or optimization bugs only show up in the context of
Ada code, although not specifically Ada related.

As for your second patch, that's really a temporary work around, since the
bugs shown are most likely general bugs not Ada specific, and that
will likely appear in complex C/C++ code as well. As such, it should
really be marked with comments explaining why the code is there, and
that it's temporary. A bugzilla PR should also be opened to remind us
that you've put in place this temporary work around.

Arno

Re: Patch ping

[Andreas Jaeger]

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

"Paolo Bonzini" <bonzini@gnu.org> writes:

>> Paolo,
>>
>> on which platform did you bootstrap this?
>
> i686-pc-linux-gnu, all languages except Ada/treelang.
>
>> It causes build errors on
>> both Linux/x86-64 and Linux/ia64 as reported by Andreas Schwab and
>> myself on the main GCC list yesterday.
>>
>> Please fix the regressions that your patch has caused.
>
> As I wrote on gcc, I don't think I can be blamed on this.  The bugs are not
> in the code I touched, but only in the code I enabled.  All I can do is
> disabling -frename-registers on the affected archs, as per the attached
> patch disable-ada-rename-regs.patch.

The GCC development rules state that you have to fix the code.  Even
if it's a latent bug that is only introduced by your changes.

> Also, I guess would have been found earlier if the Ada testsuite had tested
> all optimization levels instead of -O2 only (and it used to be -O0, which
> seems really strange to me!).  Can you please try the attached
> ada-testsuite.patch on a two-three days old gcc, on Linux/x86-64 and/or
> Linux/ia64?

I'll test the complete patch now,

Andreas

> Ok to commit both patches?
>
> Paolo
>
>
>
> Index: run_acats
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/testsuite/ada/acats/run_acats,v
> retrieving revision 1.5
> diff -u -r1.5 run_acats
> --- run_acats	8 Jan 2004 15:19:36 -0000	1.5
> +++ run_acats	19 Apr 2004 09:34:16 -0000
> @@ -51,4 +51,8 @@
>  
>  chmod +x host_gnatmake
>  
> -exec $testdir/run_all.sh "$@"
> +gccflags=-O0 $testdir/run_all.sh "$@"
> +gccflags=-O1 $testdir/run_all.sh "$@"
> +gccflags=-O2 $testdir/run_all.sh "$@"
> +gccflags=-O3 $testdir/run_all.sh "$@"
> +gccflags=-Os $testdir/run_all.sh "$@"
> Index: run_all.sh
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/testsuite/ada/acats/run_all.sh,v
> retrieving revision 1.15
> diff -u -r1.15 run_all.sh
> --- run_all.sh	17 Apr 2004 17:14:18 -0000	1.15
> +++ run_all.sh	19 Apr 2004 09:34:16 -0000
> @@ -9,8 +9,8 @@
>  # gccflags="-O3 -fomit-frame-pointer -funroll-all-loops -finline-functions"
>  # gnatflags="-gnatN"
>  
> -gccflags="-O2"
> -gnatflags="-gnatws"
> +: ${gccflags="-O2"}
> +: ${gnatflags="-gnatws"}
>  
>  target_run () {
>  $*
> @@ -78,6 +78,9 @@
>  display `type gnatmake`
>  gnatls -v >> $dir/acats.log
>  display ""
> +display gcc options are "${gccflags}"
> +display gnatmake options are "${gccflags} ${gnatflags}"
> +display ""
>  
>  display "		=== acats support ==="
>  display_noeol "Generating support files..."
> @@ -152,6 +155,9 @@
>  display " done."
>  display ""
>  display "		=== acats tests ==="
> +display ""
> +display gcc options are "${gccflags}"
> +display gnatmake options are "${gccflags} ${gnatflags}"
>  
>  if [ $# -eq 0 ]; then
>     chapters=`cd $dir/tests; echo [a-z]*`
>
> Index: i386/t-linux64
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/config/i386/t-linux64,v
> retrieving revision 1.5
> diff -u -r1.5 t-linux64
> --- i386/t-linux64	28 Nov 2002 14:47:02 -0000	1.5
> +++ i386/t-linux64	19 Apr 2004 09:43:41 -0000
> @@ -18,3 +18,5 @@
>  # because then __FRAME_END__ might not be the last thing in .eh_frame
>  # section.
>  CRTSTUFF_T_CFLAGS = -fno-omit-frame-pointer -fno-asynchronous-unwind-tables
> +
> +T_ADA_CFLAGS = -fno-rename-registers
> Index: ia64/t-ia64
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/config/ia64/t-ia64,v
> retrieving revision 1.21
> diff -u -r1.21 t-ia64
> --- ia64/t-ia64	29 Oct 2003 16:21:36 -0000	1.21
> +++ ia64/t-ia64	19 Apr 2004 09:43:41 -0000
> @@ -49,3 +49,5 @@
>  
>  # genattrtab generates very long string literals.
>  insn-attrtab.o-warn = -Wno-error
> +
> +T_ADA_CFLAGS = -fno-rename-registers

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SUSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: Patch ping

[Laurent GUERBY]

On Mon, 2004-04-19 at 11:48, Paolo Bonzini wrote:
> i686-pc-linux-gnu, all languages except Ada/treelang.
...
> Also, I guess would have been found earlier if the Ada testsuite had tested
> all optimization levels [...]

Well, if you don't include Ada in the build, the Ada testsuite flags
don't matter much :). Any reason that Ada is not running on your
i686-pc-linux-gnu machine? (most distribution have Ada lying around
these days)

I suggested some time ago to use -O2 instead of -O0 but the proposition
was rejected, hopefully it's now in.

For Ada we have to make a trade-off, running once the Ada testsuite
takes more time the whole C language test suite (28 minutes vs 17
minutes on my Athlon 2600+), so running 5 times the Ada testsuite
is not something that will go unnoticed :).

Laurent

Re: Patch ping

[Paolo Bonzini]

> > Also, I guess would have been found earlier if the Ada testsuite had
tested
> > all optimization levels [...]
>
> Well, if you don't include Ada in the build, the Ada testsuite flags
> don't matter much :).

Well, failures on ia64 will go unnoticed on i686 even if I include Ada in
the build.

> Any reason that Ada is not running on your
> i686-pc-linux-gnu machine? (most distribution have Ada lying around
> these days)

I've not spent much time looking at it, but all I could find in Debian
(apt-get install gnat) is a gcc-2.8.1 based gnat which is not picked up by
configure (its gnatmake does not invoke the installed gcc).

> For Ada we have to make a trade-off, running once the Ada testsuite
> takes more time the whole C language test suite (28 minutes vs 17
> minutes on my Athlon 2600+), so running 5 times the Ada testsuite
> is not something that will go unnoticed :).

I see.

Paolo

Re: Patch ping

[Andreas Jaeger]

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

"Paolo Bonzini" <bonzini@gnu.org> writes:

>> Paolo,
>>
>> on which platform did you bootstrap this?
>
> i686-pc-linux-gnu, all languages except Ada/treelang.
>
>> It causes build errors on
>> both Linux/x86-64 and Linux/ia64 as reported by Andreas Schwab and
>> myself on the main GCC list yesterday.
>>
>> Please fix the regressions that your patch has caused.
>
> As I wrote on gcc, I don't think I can be blamed on this.  The bugs are not
> in the code I touched, but only in the code I enabled.  All I can do is
> disabling -frename-registers on the affected archs, as per the attached
> patch disable-ada-rename-regs.patch.
>
> Also, I guess would have been found earlier if the Ada testsuite had tested
> all optimization levels instead of -O2 only (and it used to be -O0, which
> seems really strange to me!).  Can you please try the attached
> ada-testsuite.patch on a two-three days old gcc, on Linux/x86-64 and/or
> Linux/ia64?

I just tested all patches together and now is build fine for me.
Testresults will soon be on the testresults list.

But this should be fixed properly instead of worked around.

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SUSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: Patch ping

[Paolo Bonzini]

> I just tested all patches together and now is build fine for me.
> Testresults will soon be on the testresults list.


Thanks.  Of course the problem will have to be looked at more carefully, and
the workarounds are in general insufficient.  Also, on ia64-pc-linux-gnu C++
fails which is more severe, so I'm setting up a combined tree for ia64.

Paolo

Re: Patch ping

[Andreas Schwab]

"Paolo Bonzini" <bonzini@gnu.org> writes:

>> Paolo,
>>
>> on which platform did you bootstrap this?
>
> i686-pc-linux-gnu, all languages except Ada/treelang.
>
>> It causes build errors on
>> both Linux/x86-64 and Linux/ia64 as reported by Andreas Schwab and
>> myself on the main GCC list yesterday.
>>
>> Please fix the regressions that your patch has caused.
>
> As I wrote on gcc, I don't think I can be blamed on this.  The bugs are not
> in the code I touched, but only in the code I enabled.  All I can do is
> disabling -frename-registers on the affected archs, as per the attached
> patch disable-ada-rename-regs.patch.

This has nothing to do with Ada, the same error happens with libstdc++ as
well, see <http://gcc.gnu.org/ml/gcc/2004-04/msg00830.html>. It seems like
-frename-registers is generally broken, or triggers generally broken code.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Patch ping

[Paul Brook]

On Monday 19 April 2004 13:57, Andreas Schwab wrote:
> "Paolo Bonzini" <bonzini@gnu.org> writes:
> >> Paolo,
> >>
> >> on which platform did you bootstrap this?
> >
> > i686-pc-linux-gnu, all languages except Ada/treelang.
> >
> >> It causes build errors on
> >> both Linux/x86-64 and Linux/ia64 as reported by Andreas Schwab and
> >> myself on the main GCC list yesterday.
> >>
> >> Please fix the regressions that your patch has caused.
> >
> > As I wrote on gcc, I don't think I can be blamed on this.  The bugs are
> > not in the code I touched, but only in the code I enabled.  All I can do
> > is disabling -frename-registers on the affected archs, as per the
> > attached patch disable-ada-rename-regs.patch.
>
> This has nothing to do with Ada, the same error happens with libstdc++ as
> well, see <http://gcc.gnu.org/ml/gcc/2004-04/msg00830.html>. It seems like
> -frename-registers is generally broken, or triggers generally broken code.

It also breaks libstdc++ on arm/thumb.

Paul

Re: Patch ping

[Richard Earnshaw]

On Mon, 2004-04-19 at 14:16, Paul Brook wrote:
> On Monday 19 April 2004 13:57, Andreas Schwab wrote:
> > "Paolo Bonzini" <bonzini@gnu.org> writes:
> > >> Paolo,
> > >>
> > >> on which platform did you bootstrap this?
> > >
> > > i686-pc-linux-gnu, all languages except Ada/treelang.
> > >
> > >> It causes build errors on
> > >> both Linux/x86-64 and Linux/ia64 as reported by Andreas Schwab and
> > >> myself on the main GCC list yesterday.
> > >>
> > >> Please fix the regressions that your patch has caused.
> > >
> > > As I wrote on gcc, I don't think I can be blamed on this.  The bugs are
> > > not in the code I touched, but only in the code I enabled.  All I can do
> > > is disabling -frename-registers on the affected archs, as per the
> > > attached patch disable-ada-rename-regs.patch.
> >
> > This has nothing to do with Ada, the same error happens with libstdc++ as
> > well, see <http://gcc.gnu.org/ml/gcc/2004-04/msg00830.html>. It seems like
> > -frename-registers is generally broken, or triggers generally broken code.
> 
> It also breaks libstdc++ on arm/thumb.
> 
> Paul

If you are seeing this bug:

/work/rearnsha/gnu/egcs/gcc/xgcc -shared-libgcc -B/work/rearnsha/gnu/egcs/gcc/ -
nostdinc++ -L/work/rearnsha/gnu/egcs/arm-elf/libstdc++-v3/src -L/work/rearnsha/g
nu/egcs/arm-elf/libstdc++-v3/src/.libs -nostdinc -B/work/rearnsha/gnu/egcs/arm-e
lf/newlib/ -isystem /work/rearnsha/gnu/egcs/arm-elf/newlib/targ-include -isystem
 /home/rearnsha/gnusrc/egcs-cross/newlib/libc/include -B/work/rearnsha/gnu/testi
nstall/arm-elf/bin/ -B/work/rearnsha/gnu/testinstall/arm-elf/lib/ -isystem /work
/rearnsha/gnu/testinstall/arm-elf/include -isystem /work/rearnsha/gnu/testinstal
l/arm-elf/sys-include -L/work/rearnsha/gnu/egcs/ld -I/work/rearnsha/gnu/egcs/arm
-elf/libstdc++-v3/include/arm-elf -I/work/rearnsha/gnu/egcs/arm-elf/libstdc++-v3
/include -I/home/rearnsha/gnusrc/egcs-cross/libstdc++-v3/libsupc++ -O2 -g -O2 -g
 -O2 -fno-implicit-templates -Wall -W -Wwrite-strings -Wcast-qual -fdiagnostics-
show-location=once -c /home/rearnsha/gnusrc/egcs-cross/libstdc++-v3/src/complex_
io.cc -o complex_io.o
/work/rearnsha/gnu/egcs/arm-elf/libstdc++-v3/include/complex: In function `std::
basic_ostream<_CharT, _Traits>& std::operator<<(std::basic_ostream<_CharT, _Trai
ts>&, conststd::complex<_Tp>&) [with _Tp = float, _CharT = char, _Traits = std::
char_traits<char>]':
/work/rearnsha/gnu/egcs/arm-elf/libstdc++-v3/include/complex:1455: error: insn d
oes not satisfy its constraints:
(insn:HI 2559 6401 6402 47 /work/rearnsha/gnu/egcs/arm-elf/libstdc++-v3/include/
streambuf:187 (set (mem/s:SI (post_modify:SI (reg:SI 12 ip)
                (plus:SI (reg:SI 12 ip)
                    (const_int 28 [0x1c]))) [9 <variable>._vptr.basic_streambuf+
0 S4 A32])
        (reg/f:SI 3 r3 [314])) 140 {*arm_movsi_insn} (insn_list 2558 (nil))
    (expr_list:REG_DEAD (reg/f:SI 3 r3 [314])
        (expr_list:REG_INC (reg:SI 12 ip)
            (nil))))

Then I think this is because arm_legitimate_address_p is doing something
wrong.  Probably this test:

  else if ((GET_CODE (x) == POST_MODIFY || GET_CODE (x) == PRE_MODIFY)
	   && GET_MODE_SIZE (mode) <= 4
	   && arm_address_register_rtx_p (XEXP (x, 0), strict_p)
	   && GET_CODE (XEXP (x, 1)) == PLUS
	   && XEXP (XEXP (x, 1), 0) == XEXP (x, 0))

That final equivalence should probably be using rtx_equal_p(), but I
haven't tested that yet.

R.

[arm] post_modify constraint failure (was Re: Patch ping)

[Paul Brook]

> /work/rearnsha/gnu/egcs/arm-elf/libstdc++-v3/include/complex:1455: error:
> insn d oes not satisfy its constraints:
> (insn:HI 2559 6401 6402 47
> /work/rearnsha/gnu/egcs/arm-elf/libstdc++-v3/include/ streambuf:187 (set
> (mem/s:SI (post_modify:SI (reg:SI 12 ip)
>                 (plus:SI (reg:SI 12 ip)
>                     (const_int 28 [0x1c]))) [9
> <variable>._vptr.basic_streambuf+ 0 S4 A32])
>         (reg/f:SI 3 r3 [314])) 140 {*arm_movsi_insn} (insn_list 2558 (nil))
>     (expr_list:REG_DEAD (reg/f:SI 3 r3 [314])
>         (expr_list:REG_INC (reg:SI 12 ip)
>             (nil))))
>
> Then I think this is because arm_legitimate_address_p is doing something
> wrong.  Probably this test:
>
>   else if ((GET_CODE (x) == POST_MODIFY || GET_CODE (x) == PRE_MODIFY)
> 	   && GET_MODE_SIZE (mode) <= 4
> 	   && arm_address_register_rtx_p (XEXP (x, 0), strict_p)
> 	   && GET_CODE (XEXP (x, 1)) == PLUS
> 	   && XEXP (XEXP (x, 1), 0) == XEXP (x, 0))
>
> That final equivalence should probably be using rtx_equal_p(), but I
> haven't tested that yet.

It does fix the problem. Patch below.

Tested with cross-compiler to arm-none-elf.
Ok?

Paul

2004-04-20  Paul Brook  <paul@codesourcery.com>

	* config/arm/arm.c (arm_legitimate_address_p): Use rtx_equal_p.

Index: config/arm/arm.c
===================================================================
RCS file: /var/cvsroot/gcc-cvs/gcc/gcc/config/arm/arm.c,v
retrieving revision 1.347
diff -u -p -r1.347 arm.c
--- a/config/arm/arm.c	20 Apr 2004 11:28:08 -0000	1.347
+++ b/config/arm/arm.c	20 Apr 2004 12:17:13 -0000
@@ -2994,7 +2994,7 @@ arm_legitimate_address_p (enum machine_m
 	   && GET_MODE_SIZE (mode) <= 4
 	   && arm_address_register_rtx_p (XEXP (x, 0), strict_p)
 	   && GET_CODE (XEXP (x, 1)) == PLUS
-	   && XEXP (XEXP (x, 1), 0) == XEXP (x, 0))
+	   && rtx_equal_p (XEXP (XEXP (x, 1), 0), XEXP (x, 0)))
     return arm_legitimate_index_p (mode, XEXP (XEXP (x, 1), 1), outer,
 				   strict_p);
 

Re: [arm] post_modify constraint failure (was Re: Patch ping)

[Richard Earnshaw]

On Tue, 2004-04-20 at 15:12, Paul Brook wrote:
> > /work/rearnsha/gnu/egcs/arm-elf/libstdc++-v3/include/complex:1455: error:
> > insn d oes not satisfy its constraints:
> > (insn:HI 2559 6401 6402 47
> > /work/rearnsha/gnu/egcs/arm-elf/libstdc++-v3/include/ streambuf:187 (set
> > (mem/s:SI (post_modify:SI (reg:SI 12 ip)
> >                 (plus:SI (reg:SI 12 ip)
> >                     (const_int 28 [0x1c]))) [9
> > <variable>._vptr.basic_streambuf+ 0 S4 A32])
> >         (reg/f:SI 3 r3 [314])) 140 {*arm_movsi_insn} (insn_list 2558 (nil))
> >     (expr_list:REG_DEAD (reg/f:SI 3 r3 [314])
> >         (expr_list:REG_INC (reg:SI 12 ip)
> >             (nil))))
> >
> > Then I think this is because arm_legitimate_address_p is doing something
> > wrong.  Probably this test:
> >
> >   else if ((GET_CODE (x) == POST_MODIFY || GET_CODE (x) == PRE_MODIFY)
> > 	   && GET_MODE_SIZE (mode) <= 4
> > 	   && arm_address_register_rtx_p (XEXP (x, 0), strict_p)
> > 	   && GET_CODE (XEXP (x, 1)) == PLUS
> > 	   && XEXP (XEXP (x, 1), 0) == XEXP (x, 0))
> >
> > That final equivalence should probably be using rtx_equal_p(), but I
> > haven't tested that yet.
> 
> It does fix the problem. Patch below.
> 
> Tested with cross-compiler to arm-none-elf.
> Ok?
> 
> Paul
> 
> 2004-04-20  Paul Brook  <paul@codesourcery.com>
> 
> 	* config/arm/arm.c (arm_legitimate_address_p): Use rtx_equal_p.
> 

OK.

R.

Re: Patch ping

[Richard Earnshaw]

On Mon, 2004-04-19 at 13:57, Andreas Schwab wrote:
> "Paolo Bonzini" <bonzini@gnu.org> writes:
> 
> >> Paolo,
> >>
> >> on which platform did you bootstrap this?
> >
> > i686-pc-linux-gnu, all languages except Ada/treelang.
> >
> >> It causes build errors on
> >> both Linux/x86-64 and Linux/ia64 as reported by Andreas Schwab and
> >> myself on the main GCC list yesterday.
> >>
> >> Please fix the regressions that your patch has caused.
> >
> > As I wrote on gcc, I don't think I can be blamed on this.  The bugs are not
> > in the code I touched, but only in the code I enabled.  All I can do is
> > disabling -frename-registers on the affected archs, as per the attached
> > patch disable-ada-rename-regs.patch.
> 
> This has nothing to do with Ada, the same error happens with libstdc++ as
> well, see <http://gcc.gnu.org/ml/gcc/2004-04/msg00830.html>. It seems like
> -frename-registers is generally broken, or triggers generally broken code.

Not to mention the fact that it also appears to be a major CPU hog. 
Depending on the platform this can cause up to about 4% increase in
compile times.

R.

Disable -frename-registers

[Paolo Bonzini]

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

> It seems like -frename-registers is generally
> broken, or triggers generally broken code.

What about the attached patch?  Would it be ok to commit as obvious?  

Sorry for the breakage.

Paolo

2004-04-19  Paolo Bonzini  <bonzini@gnu.org>

        Revert part of 2004-04-17 change to 
        * toplev.c (flag_rename_registers): Initialize to 0.
        * doc/invoke.texi (Optimize options): Move -frename-registers
        to "Not triggered by any -O level" section.  Adjust commentary
        accordingly.

[-- Attachment #2: disable-regrename.patch --]
[-- Type: application/octet-stream, Size: 2600 bytes --]

Index: toplev.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/toplev.c,v
retrieving revision 1.893
diff -u -r1.893 toplev.c
--- toplev.c	17 Apr 2004 06:53:43 -0000	1.893
+++ toplev.c	19 Apr 2004 13:28:29 -0000
@@ -268,8 +268,9 @@
 
 /* Nonzero if registers should be renamed.  When
    flag_rename_registers == AUTODETECT_FLAG_VAR_TRACKING it will be set
-   according to optimize and default_debug_hooks in process_options ().  */
-int flag_rename_registers = AUTODETECT_FLAG_VAR_TRACKING;
+   according to optimize and default_debug_hooks in process_options (),
+   but we do not do this yet because it triggers aborts in flow.c.  */
+int flag_rename_registers = 0;
 int flag_cprop_registers = 0;
 
 /* Nonzero for -pedantic switch: warn about anything
Index: doc/invoke.texi
===================================================================
RCS file: /cvs/gcc/gcc/gcc/doc/invoke.texi,v
retrieving revision 1.447
diff -u -r1.447 invoke.texi
--- doc/invoke.texi	18 Apr 2004 23:17:28 -0000	1.447
+++ doc/invoke.texi	19 Apr 2004 13:28:32 -0000
@@ -4342,19 +4342,6 @@
 
 Enabled at levels @option{-O2}, @option{-O3}.
 
-@item -frename-registers
-@opindex frename-registers
-Attempt to avoid false dependencies in scheduled code by making use
-of registers left over after register allocation.  This optimization
-will most benefit processors with lots of registers.  Depending on the
-debug information format adopted by the target, however, it can
-make debugging impossible, since variables will no longer stay in
-a ``home register''.
-
-Enabled at levels @option{-O}, @option{-O2}, @option{-O3}, @option{-Os},
-on targets where the default format for debugging information supports
-variable tracking.
-
 @item -fweb
 @opindex fweb
 Constructs webs as commonly used for register allocation purposes and assign
@@ -4574,6 +4561,17 @@
 using the knowledge about the value of the denominator.
 
 Enabled with @option{-profile-generate} and @option{-profile-use}.
+
+@item -frename-registers
+@opindex frename-registers
+Attempt to avoid false dependencies in scheduled code by making use
+of registers left over after register allocation.  This optimization
+will most benefit processors with lots of registers.  Depending on the
+debug information format adopted by the target, however, it can
+make debugging impossible, since variables will no longer stay in
+a ``home register''.
+
+Not enabled by default at any level because it has known bugs.
 
 @item -fnew-ra
 @opindex fnew-ra

Re: Disable -frename-registers

[Geoff Keating]

"Paolo Bonzini" <bonzini@gnu.org> writes:

> > It seems like -frename-registers is generally
> > broken, or triggers generally broken code.
> 
> What about the attached patch?  Would it be ok to commit as obvious?  
> 
> Sorry for the breakage.
> 
> Paolo
> 
> 2004-04-19  Paolo Bonzini  <bonzini@gnu.org>
> 
>         Revert part of 2004-04-17 change to 
>         * toplev.c (flag_rename_registers): Initialize to 0.
>         * doc/invoke.texi (Optimize options): Move -frename-registers
>         to "Not triggered by any -O level" section.  Adjust commentary
>         accordingly.

This is OK, but please also file a bugzilla bug report about the
problems.

-- 
- Geoffrey Keating <geoffk@geoffk.org>

Re: Patch ping

[Andreas Jaeger]

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

Geoff Keating <geoffk@geoffk.org> writes:

> "Paolo Bonzini" <bonzini@gnu.org> writes:
>
>> http://gcc.gnu.org/ml/gcc-patches/2004-04/msg00384.html
>> Enables -fweb at -O2 and -frename-registers at -O1 if variable
>> tracking is available for the target's default debugging information
>> format.
>> 
>> Ok for mainline?
>
> This is OK, assuming you bootstrapped it on at least one platform.

Paolo,

on which platform did you bootstrap this?  It causes build errors on
both Linux/x86-64 and Linux/ia64 as reported by Andreas Schwab and
myself on the main GCC list yesterday.

Please fix the regressions that your patch has caused.

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]