Re: stabilization issues ppc-linux

Re: stabilization issues ppc-linux

[Franz Sirl]

Am Sun, 10 Jan 1999 schrieb Jeffrey A Law:
>Well, I tried bootstrapping the compiler using the assembler from the latest
>binutils snapshot.  No joy.  Linking static seems to work around that problem,
>but then stage2 cc1 fails in various weird (core dumps at a weird address with
>no useable backtrace).
>
>I'm not going to waste any more time on this platform.  Hopefully Franz, Geoff,
>Gary and others will continue to test on ppc-linux and keep the compiler
>(slowly) moving forward on that platform.

I would love to help, but currently cvs-egcs won't even pass the build of
stage1.
It fails with:
_eprintf
_bb
.../../../egcs/gcc/libgcc2.c: In function `__bb_exit_func':
.../../../egcs/gcc/libgcc2.c:1599: warning: implicit declaration of function `time'
.../../../egcs/gcc/libgcc2.c:1609: warning: comparison between signed and unsigned
.../../../egcs/gcc/libgcc2.c:1637: warning: comparison between signed and unsigned
.../../../egcs/gcc/varasm.c:3259: Internal compiler error in function decode_rtx_const
Please submit a full bug report to `egcs-bugs@cygnus.com'.
See <URL: http://egcs.cygnus.com/faq.html#bugreport > for details.
make[4]: *** [libgcc2.a] Error 1
make[4]: Leaving directory `/sda9/src/e/obj/cvs1/gcc'
make[3]: *** [stmp-multilib-sub] Error 2
make[3]: Leaving directory `/sda9/src/e/obj/cvs1/gcc'
make[2]: *** [stmp-multilib] Error 1
make[2]: Leaving directory `/sda9/src/e/obj/cvs1/gcc'
make[1]: *** [bootstrap-lean] Error 2
make[1]: Leaving directory `/sda9/src/e/obj/cvs1/gcc'
make: *** [bootstrap-lean] Error 2

I assume this is caused by the recent CONSTANT_P_RTX changes by rth. I removed
the CONSTANT_P_RTX handling from the rs6000 backend, but this didn't change
anything.

Cannot insert breakpoint 1:
Temporarily disabling shared library breakpoints:
1 2 
GNU C version egcs-2.93.00 19990110 (gcc2 ss-980609 experimental) (ppc-redhat-linux) compiled by GNU C version egcs-2.91.60 19981201 (egcs-1.1.1 release).
.../../../egcs/gcc/libgcc2.c: In function `__bb_exit_func':
.../../../egcs/gcc/libgcc2.c:1599: warning: implicit declaration of function `time'
.../../../egcs/gcc/libgcc2.c:1609: warning: comparison between signed and unsigned
.../../../egcs/gcc/libgcc2.c:1637: warning: comparison between signed and unsigned

Breakpoint 5, decode_rtx_const (mode=SImode, x=0x1ca64a0, value=0x7fffeee8) at ../../../egcs/gcc/varasm.c:3259
(xxgdb) undisplay
(xxgdb) print *x
$1 = {
  code = CONSTANT_P_RTX, 
  mode = SImode, 
  jump = 0, 
  call = 0, 
  unchanging = 0, 
  volatil = 0, 
  in_struct = 0, 
  used = 0, 
  integrated = 0, 
  frame_related = 0, 
  fld = {{
      rtwint = 29837736, 
      rtint = 29837736, 
      rtstr = 0x1c749a8 "", 
      rtx = 0x1c749a8, 
      rtvec = 0x1c749a8, 
      rttype = 29837736, 
      rt_addr_diff_vec_flags = {
        min_align = 1, 
        base_after_vec = 1, 
        min_after_vec = 1, 
        max_after_vec = 0, 
        min_after_base = 0, 
        max_after_base = 0, 
        offset_unsigned = 1, 
         = 3, 
        scale = 73
      },
      rtbit = 0x1c749a8, 
      rttree = 0x1c749a8
    }}
}
(xxgdb) print x
$2 = 0x1ca64a0
(xxgdb) pr
(constant_p_rtx:SI (reg/v:SI 829))
(xxgdb) ptc
There is no member named common.
(xxgdb) pt
 <error_mark 0x1ca64a0>
(xxgdb) 

I'm appending the CONSTANT_P_RTX changes, maybe they are helpful (the bug
happens with and without them). Please tell me if you need to know something
else.

Franz.


? diff10
? diff9
Index: rs6000.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/config/rs6000/rs6000.c,v
retrieving revision 1.52
diff -u -p -r1.52 rs6000.c
--- rs6000.c	1999/01/06 19:42:20	1.52
+++ rs6000.c	1999/01/10 14:17:35
@@ -497,8 +497,7 @@ short_cint_operand (op, mode)
      enum machine_mode mode ATTRIBUTE_UNUSED;
 {
   return ((GET_CODE (op) == CONST_INT
-	   && (unsigned HOST_WIDE_INT) (INTVAL (op) + 0x8000) < 0x10000)
-	  || GET_CODE (op) == CONSTANT_P_RTX);
+	   && (unsigned HOST_WIDE_INT) (INTVAL (op) + 0x8000) < 0x10000));
 }
 
 /* Similar for a unsigned D field.  */
@@ -509,8 +508,7 @@ u_short_cint_operand (op, mode)
      enum machine_mode mode ATTRIBUTE_UNUSED;
 {
   return ((GET_CODE (op) == CONST_INT
-	   && (INTVAL (op) & (~ (HOST_WIDE_INT) 0xffff)) == 0)
-	  || GET_CODE (op) == CONSTANT_P_RTX);
+	   && (INTVAL (op) & (~ (HOST_WIDE_INT) 0xffff)) == 0));
 }
 
 /* Return 1 if OP is a CONST_INT that cannot fit in a signed D field.  */
@@ -598,7 +596,6 @@ reg_or_cint_operand (op, mode)
     enum machine_mode mode;
 {
      return (GET_CODE (op) == CONST_INT
-	     || GET_CODE (op) == CONSTANT_P_RTX
 	     || gpc_reg_operand (op, mode));
 }
 
@@ -883,8 +880,7 @@ logical_operand (op, mode)
   return (gpc_reg_operand (op, mode)
 	  || (GET_CODE (op) == CONST_INT
 	      && ((INTVAL (op) & (~ (HOST_WIDE_INT) 0xffff)) == 0
-		  || (INTVAL (op) & 0xffff) == 0))
-	  || GET_CODE (op) == CONSTANT_P_RTX);
+		  || (INTVAL (op) & 0xffff) == 0)));
 }
 
 /* Return 1 if C is a constant that is not a logical operand (as
@@ -1110,6 +1106,10 @@ input_operand (op, mode)
   if (memory_operand (op, mode))
     return 1;
 
+  /* Only a tiny bit of handling for CONSTANT_P_RTX is necessary.  */
+  if (GET_CODE (op) == CONST && GET_CODE (XEXP (op, 0)) == CONSTANT_P_RTX)
+    return 1;
+
   /* For floating-point, easy constants are valid.  */
   if (GET_MODE_CLASS (mode) == MODE_FLOAT
       && CONSTANT_P (op)
@@ -1119,7 +1119,6 @@ input_operand (op, mode)
   /* Allow any integer constant.  */
   if (GET_MODE_CLASS (mode) == MODE_INT
       && (GET_CODE (op) == CONST_INT
-	  || GET_CODE (op) == CONSTANT_P_RTX
 	  || GET_CODE (op) == CONST_DOUBLE))
     return 1;
 
Index: rs6000.h
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/config/rs6000/rs6000.h,v
retrieving revision 1.33
diff -u -p -r1.33 rs6000.h
--- rs6000.h	1998/12/16 21:12:11	1.33
+++ rs6000.h	1999/01/10 14:17:42
@@ -3169,15 +3169,15 @@ do {									\
 /* Define the codes that are matched by predicates in rs6000.c.  */
 
 #define PREDICATE_CODES						\
-  {"short_cint_operand", {CONST_INT, CONSTANT_P_RTX}},		\
-  {"u_short_cint_operand", {CONST_INT, CONSTANT_P_RTX}},	\
+  {"short_cint_operand", {CONST_INT}},				\
+  {"u_short_cint_operand", {CONST_INT}},			\
   {"non_short_cint_operand", {CONST_INT}},			\
   {"gpc_reg_operand", {SUBREG, REG}},				\
   {"cc_reg_operand", {SUBREG, REG}},				\
-  {"reg_or_short_operand", {SUBREG, REG, CONST_INT, CONSTANT_P_RTX}}, \
+  {"reg_or_short_operand", {SUBREG, REG, CONST_INT}}, 		\
   {"reg_or_neg_short_operand", {SUBREG, REG, CONST_INT}},	\
-  {"reg_or_u_short_operand", {SUBREG, REG, CONST_INT, CONSTANT_P_RTX}}, \
-  {"reg_or_cint_operand", {SUBREG, REG, CONST_INT, CONSTANT_P_RTX}}, \
+  {"reg_or_u_short_operand", {SUBREG, REG, CONST_INT}}, 	\
+  {"reg_or_cint_operand", {SUBREG, REG, CONST_INT}}, 		\
   {"got_operand", {SYMBOL_REF, CONST, LABEL_REF}},		\
   {"got_no_const_operand", {SYMBOL_REF, LABEL_REF}},		\
   {"easy_fp_constant", {CONST_DOUBLE}},				\
@@ -3186,12 +3186,11 @@ do {									\
   {"volatile_mem_operand", {MEM}},				\
   {"offsettable_addr_operand", {REG, SUBREG, PLUS}},		\
   {"mem_or_easy_const_operand", {SUBREG, MEM, CONST_DOUBLE}},	\
-  {"add_operand", {SUBREG, REG, CONST_INT, CONSTANT_P_RTX}},	\
+  {"add_operand", {SUBREG, REG, CONST_INT}},			\
   {"non_add_cint_operand", {CONST_INT}},			\
-  {"and_operand", {SUBREG, REG, CONST_INT, CONSTANT_P_RTX}},	\
-  {"and64_operand", {SUBREG, REG, CONST_INT, CONSTANT_P_RTX,	\
-		     CONST_DOUBLE}},				\
-  {"logical_operand", {SUBREG, REG, CONST_INT, CONSTANT_P_RTX}}, \
+  {"and_operand", {SUBREG, REG, CONST_INT}},			\
+  {"and64_operand", {SUBREG, REG, CONST_INT, CONST_DOUBLE}},	\
+  {"logical_operand", {SUBREG, REG, CONST_INT}}, 		\
   {"non_logical_cint_operand", {CONST_INT}},			\
   {"mask_operand", {CONST_INT}},				\
   {"mask64_operand", {CONST_INT, CONST_DOUBLE}},		\
@@ -3199,7 +3198,7 @@ do {									\
   {"fpmem_operand", {REG}},					\
   {"call_operand", {SYMBOL_REF, REG}},				\
   {"current_file_function_operand", {SYMBOL_REF}},		\
-  {"input_operand", {SUBREG, MEM, REG, CONST_INT, CONSTANT_P_RTX, \
+  {"input_operand", {SUBREG, MEM, REG, CONST_INT, 		\
 		     CONST_DOUBLE, SYMBOL_REF}}, 		\
   {"load_multiple_operation", {PARALLEL}},			\
   {"store_multiple_operation", {PARALLEL}},			\
Index: rs6000.md
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/config/rs6000/rs6000.md,v
retrieving revision 1.33
diff -u -p -r1.33 rs6000.md
--- rs6000.md	1998/12/30 19:37:53	1.33
+++ rs6000.md	1999/01/10 14:17:51
@@ -5420,8 +5420,7 @@
       && !flag_pic
       && CONSTANT_P (operands[1])
       && GET_CODE (operands[1]) != HIGH
-      && GET_CODE (operands[1]) != CONST_INT
-      && GET_CODE (operands[1]) != CONSTANT_P_RTX)
+      && GET_CODE (operands[1]) != CONST_INT)
     {
       rtx target = (reload_completed || reload_in_progress)
 			? operands[0] : gen_reg_rtx (SImode);
@@ -5475,7 +5474,6 @@
   if ((!TARGET_WINDOWS_NT || DEFAULT_ABI != ABI_NT)
       && CONSTANT_P (operands[1])
       && GET_CODE (operands[1]) != CONST_INT
-      && GET_CODE (operands[1]) != CONSTANT_P_RTX
       && GET_CODE (operands[1]) != HIGH
       && ! LEGITIMATE_CONSTANT_POOL_ADDRESS_P (operands[1]))
     {
@@ -5574,8 +5572,7 @@
     operands[1] = force_reg (HImode, operands[1]);
 
   if (CONSTANT_P (operands[1])
-      && GET_CODE (operands[1]) != CONST_INT
-      && GET_CODE (operands[1]) != CONSTANT_P_RTX)
+      && GET_CODE (operands[1]) != CONST_INT)
     {
       operands[1] = force_const_mem (HImode, operands[1]);
       if (! memory_address_p (HImode, XEXP (operands[1], 0))
@@ -5611,8 +5608,7 @@
     operands[1] = force_reg (QImode, operands[1]);
 
   if (CONSTANT_P (operands[1])
-      && GET_CODE (operands[1]) != CONST_INT
-      && GET_CODE (operands[1]) != CONSTANT_P_RTX)
+      && GET_CODE (operands[1]) != CONST_INT)
     {
       operands[1] = force_const_mem (QImode, operands[1]);
       if (! memory_address_p (QImode, XEXP (operands[1], 0))
@@ -6016,7 +6012,6 @@
       && CONSTANT_P (operands[1])
 #if HOST_BITS_PER_WIDE_INT == 32
       && GET_CODE (operands[1]) != CONST_INT
-      && GET_CODE (operands[1]) != CONSTANT_P_RTX
 #endif
       && ! easy_fp_constant (operands[1], DImode)
       && ! LEGITIMATE_CONSTANT_POOL_ADDRESS_P (operands[1]))

Re: stabilization issues ppc-linux

[David Edelsohn]

>>>>> Richard Henderson writes:

Richard> On Mon, Jan 11, 1999 at 01:15:24AM -0500, David Edelsohn wrote:
>> If all these rs6000 changes are correct, I do not object to
>> applying them.  However, if you make changes to movsi and movdi, you need
>> to make consistent changes to movhi and movqi as well.

Richard> No, because only moves for Pmode need tweeking.

	Well, I did not understand this when we originally were making the
previous changes and *I* added CONSTANT_P_RTX tests to movhi and movqi.
If those tests are not needed whatsoever and do not need to be replaced as
with movsi and movdi, that is great.  All I am saying is that the same
code should be ripped out everywhere if it is ripped out anywhere. 

David

Re: stabilization issues ppc-linux

[Richard Henderson]

On Sun, Jan 10, 1999 at 04:57:08PM -0700, Jeffrey A Law wrote:
> However, we don't want to do this without detailed analysis of the bug.  It is
> wrong to remove code simply because it causes some code to fail.  Before
> taking this kind of action we have to have a very clear understanding of the
> bug to be sure that removing a hunk of code is clearly the right thing to do.
> 
> Anything less is just papering over a bug instead of trying to fix it.

Indeed, and would have been in this case as well.  The bug existed
before, but for the vaguaries of a hash table didn't seem to manifest.

CSE must not prefer to retain the constant_p_rtx.  The first attached
patch accomplishes this.

The second is a slight refinement of Franz' rs6000 patch.  Kenner's
assertion of no needed backend changes was not wholly correct -- some
support may still be necessary.  To be fair, many of the older ports
do work without modification.


r~

stabilization issues ppc-linux

[Jeffrey A Law]

Well, I tried bootstrapping the compiler using the assembler from the latest
binutils snapshot.  No joy.  Linking static seems to work around that problem,
but then stage2 cc1 fails in various weird (core dumps at a weird address with
no useable backtrace).

I'm not going to waste any more time on this platform.  Hopefully Franz, Geoff,
Gary and others will continue to test on ppc-linux and keep the compiler
(slowly) moving forward on that platform.

jeff

Re: stabilization issues ppc-linux

[Jeffrey A Law]

  In message < 4.1.19990112105131.038e31f0@mail.lauterbach.com >you write:
  > Ok, with these 2 patches cvs-egcs bootstraps fine again on a glibc-2.0.108
OK.  Good.

  > (I wish there was anonymous cvs for binutils/gas too).
It's in the works, same with gdb.  I don't know the rollout plans though.


  > Jeff, your initial email suggested that you had problems bootstrapping even
  > with a recent gas-snapshot. Did you use non-standard BOOT_CFLAGS for that?
I was going to try another bootstrap after rth's recent fixes, but the
ppc-linux box I've been using isn't responding.

I've tried normal bootstraps, bootstraps with -O2 -static and a -O0 -static,
each failed.  Though the -static ones got the furthest.

I suspect at least part of the problem is the lame-o 1.99 glibc variant on
that box.


jeff

Re: stabilization issues ppc-linux

[Franz Sirl]

At 05:59 11.01.99 , Richard Henderson wrote:
>On Sun, Jan 10, 1999 at 04:57:08PM -0700, Jeffrey A Law wrote:
>> However, we don't want to do this without detailed analysis of the bug.  
>It is
>> wrong to remove code simply because it causes some code to fail.  Before
>> taking this kind of action we have to have a very clear understanding of the
>> bug to be sure that removing a hunk of code is clearly the right thing
to do.
>> 
>> Anything less is just papering over a bug instead of trying to fix it.
>
>Indeed, and would have been in this case as well.  The bug existed
>before, but for the vaguaries of a hash table didn't seem to manifest.
>
>CSE must not prefer to retain the constant_p_rtx.  The first attached
>patch accomplishes this.
>
>The second is a slight refinement of Franz' rs6000 patch.  Kenner's
>assertion of no needed backend changes was not wholly correct -- some
>support may still be necessary.  To be fair, many of the older ports
>do work without modification.

Ok, with these 2 patches cvs-egcs bootstraps fine again on a glibc-2.0.108
machine with gas-990107 (I wish there was anonymous cvs for binutils/gas too).

I haven't run the testsuite yet, cause this machine is so _slow_
(PPC601/75), I'll let it run today.

Jeff, your initial email suggested that you had problems bootstrapping even
with a recent gas-snapshot. Did you use non-standard BOOT_CFLAGS for that?

Franz.

Re: stabilization issues ppc-linux

[David Edelsohn]

	I have no idea what is wrong on ppc-linux.  Nothing has been
changed local to the port during the time that the breakage occurred other
than Meissner's ASM_OUTPUT_ALIGNED_LOCAL move to .sbss from .sdata.

	Did any of the SVR4 changes unintentionally affect PPC when they
were intended to i386?  I have not noticed any AIX/XCOFF problems.  This
isn't a Linux kernel or glibc by product?

	My only other suggestion is to use CVS checkout on specific dates
to divide-and-conquer finding the date of the offending change.  I think
in addition to freezes we need to be a little more aggressive about
reverting changes which break the compiler until the original author
corrects the error.

David

Re: stabilization issues ppc-linux

[David Edelsohn]

	If all these rs6000 changes are correct, I do not object to
applying them.  However, if you make changes to movsi and movdi, you need
to make consistent changes to movhi and movqi as well.

	Would someone please explain to me beyond the ChangeLog entries
*WHY* this change is correct?  Especially after we went through all of the
trouble of rushing these changes into egcs-1.1 release?

Thanks, David

Re: stabilization issues ppc-linux

[Joern Rennecke]

> One might argue that it should treat CONSTANT_P_RTX like ADDRESSOF -- delay
> committal until after CSE has finished.  Or even later.  Unlike ADDRESSOF
> gcse could expose more constants via global constant propagation.

It should be OK to commit to the result one when this can be determined -
this can allow to remove some dependent code.
There should also be a cse and jump pass after we commit to values for
CONSTANT_P_RTX.
So maybe we should do this after gcse, just before the the folowing jump
optimizer - but the replacement of CONSTANT_P_RTX itself must not depend on
flag_gcse, of course.

Re: stabilization issues ppc-linux

[Richard Henderson]

On Mon, Jan 11, 1999 at 01:15:24AM -0500, David Edelsohn wrote:
> 	If all these rs6000 changes are correct, I do not object to
> applying them.  However, if you make changes to movsi and movdi, you need
> to make consistent changes to movhi and movqi as well.

No, because only moves for Pmode need tweeking.

> 	Would someone please explain to me beyond the ChangeLog entries
> *WHY* this change is correct?  Especially after we went through all of the
> trouble of rushing these changes into egcs-1.1 release?

The rs6000 changes are because Kenner came up with a slightly
better way to implement __builtin_constant_p, which has a
different form than originally implemented.  So most of the
backend changes that were done before should be undone.


r~

Re: stabilization issues ppc-linux

[Jeffrey A Law]

  In message < 19990110205928.A6380@dot.cygnus.com >you write:
  > CSE must not prefer to retain the constant_p_rtx.  The first attached
  > patch accomplishes this.
One might argue that it should treat CONSTANT_P_RTX like ADDRESSOF -- delay
committal until after CSE has finished.  Or even later.  Unlike ADDRESSOF
gcse could expose more constants via global constant propagation.

But that can always be a future improvement.


jeff

Re: stabilization issues ppc-linux

[Jeffrey A Law]

  In message < 9901102304.AA26846@marc.watson.ibm.com >you write:
  > in addition to freezes we need to be a little more aggressive about
  > reverting changes which break the compiler until the original author
  > corrects the error.
We certainly reserve the right to do this.

However, we don't want to do this without detailed analysis of the bug.  It is
wrong to remove code simply because it causes some code to fail.  Before
taking this kind of action we have to have a very clear understanding of the
bug to be sure that removing a hunk of code is clearly the right thing to do.

Anything less is just papering over a bug instead of trying to fix it.  Take
for example HJ's desire to turn off regmove in egcs-1.0.? because the compiler
was aborting or generating incorrect code in certain circumstances.

Turning off regmove would have avoided that instance of the problem, but it
would have been absolutely the wrong thing to do since the problem actually
had nothing to do with regmove.  In fact, once we had a good handle on the
problem it was fairly easy to construct a testcase which failed with regmove
disabled. 

It has been my experience that when we analyze the bug, 99% of the time
the person responsible for whatever the real problem is will fix it (if asked).


jeff