One bug in unroll.c

One bug in unroll.c

[H.J. Lu]

After applying this patch, 920625-1.c seems ok now. I have no
idea how the new alias code works. But I believe something is
wrong. I think if the current aliase code is released in
egcs as is, the egcs project may start on a wrong foot. To the
least, the first public release of egcs should not be worse
than gcc2 ss-970802, which is egcs based upon.


Thanks.


-- 
H.J. Lu (hjl@gnu.ai.mit.edu)
---
--- unroll.c.orig	Sat Aug 23 14:51:27 1997
+++ unroll.c	Sat Aug 23 15:00:22 1997
@@ -2860,9 +2860,13 @@
 		     register to hold the split value of the DEST_ADDR giv.
 		     Emit insn to initialize its value before loop start.  */
 
+#if 0
 		  rtx tem = gen_reg_rtx (v->mode);
-		  record_base_value (REGNO (tem), v->add_val);
 		  v->unrolled = 1;
+#else
+		  tem = gen_reg_rtx (v->mode);
+#endif
+		  record_base_value (REGNO (tem), v->add_val);
 
 		  /* If the address giv has a constant in its new_reg value,
 		     then this constant can be pulled out and put in value,

Re: One bug in unroll.c

[Toon Moene]

>  Try this patch, which was in my version of the alias code
>  but not egcs.  As far as I know the bug fixed by this
>  patch does not affect RISC machines; it was discovered
>  several months ago in Fortran testing on (I think) m68k.

Close, but no cigar :-)  The code that exposed this bug circulated  
on g77-dev, but was C code.  Furthermore, one of the distingiushing  
facts about this bug was that it *could never have been found on an  
m68k*, because config/m68k/m68k.h doesn't define ADDRESS_COST.

This is what I wrote to g77-dev on the 11th of April, in reply to  
Craig's request to look into his solution of this problem:

QUOTE
	* unroll.c (find_splittable_givs): Must create a new
	register if the mult_val and add_val fields don't
	agree.

Aaah, I finally see why I couldn't reproduce this problem - "same"  
in the sense used in unroll.c means "combined" by combine_givs in  
loop.c - and it will only do that if the GIVs are *identical* _or_  
the macro ADDRESS_COST is defined, which is used to determine  
"expensive" in:

/* If G2 can be expressed as a function of G1 and that function is valid
   as an address and no more expensive than using a register for G2,
   the expression of G2 in terms of G1 can be used.  */

Well, ADDRESS_COST is not defined for m68k systems, but is for i386  
and alpha ...

check on mult_val and add_val in unroll.c:  Both GIVs are not  
necessary the "same" as implied by this test (i.e. identical), but  
one is computable from the other (in a cheap way).
UNQUOTE

In fact, this points out that using a member of a struct called  
"same" in this case is highly misleading and should be corrected (or  
ADDRESS_COST should be eliminated).

Cheers,
Toon.

Re: One bug in unroll.c

[John Carr]

> BTW, how different is your my version of the alias code from
> the one in egcs? What other bug fixes should I know?

I think that was the only bug fix missing from the egcs version (there
have been only one or two bug fixes total).  There are some missing
features -- "restrict" is the most interesting one -- which I will be
merging soon.

Re: One bug in unroll.c

[H.J. Lu]

> 
> 
> Try this patch, which was in my version of the alias code but not egcs.  As
> far as I know the bug fixed by this patch does not affect RISC machines; it
> was discovered several months ago in Fortran testing on (I think) m68k.
> 
> 

Yes. That fixes 920625-1.c. I will try it on other test cases. Thanks
a lot.

BTW, how different is your my version of the alias code from
the one in egcs? What other bug fixes should I know?

Thanks.


H.J.

Re: One bug in unroll.c

[Jeffrey A Law]

  In message <199708232227.SAA13018@jfc.>you write:
  > 
  > Sat Aug 23 18:19:40 1997  John F. Carr  <jfc@mit.edu>
  > 
  > 	* unroll.c (find_splittable_givs): If ADDRESS_COST is defined
  > 	combine_givs_p may return true for registers which are different
  > 	but related.  Check that mult_val and add_val are equal before
  > 	sharing registers.
I've installed this patch, but without the #ifdef ADDRESS_COST
conditional.

Jeff

Re: One bug in unroll.c

[John Carr]

Try this patch, which was in my version of the alias code but not egcs.  As
far as I know the bug fixed by this patch does not affect RISC machines; it
was discovered several months ago in Fortran testing on (I think) m68k.


Sat Aug 23 18:19:40 1997  John F. Carr  <jfc@mit.edu>

	* unroll.c (find_splittable_givs): If ADDRESS_COST is defined
	combine_givs_p may return true for registers which are different
	but related.  Check that mult_val and add_val are equal before
	sharing registers.

*** ../gcc/unroll.c	Sat Jun 28 16:19:03 1997
--- unroll.c	Thu Aug 21 18:49:09 1997
***************
*** 2830,2845 ****
  	      else if (unroll_type != UNROLL_COMPLETELY
  		       && v->giv_type == DEST_ADDR
  		       && v->same && v->same->giv_type == DEST_ADDR
! 		       && v->same->unrolled
! #ifdef ADDRESS_COST
! 		       /* combine_givs_p may return true when ADDRESS_COST
! 			  is defined even if the multiply and add values
! 			  are not equal.  To share a register here the
! 			  values must be equal, not only related.  */
! 		       && rtx_equal_p (v->same->mult_val, v->mult_val)
! 		       && rtx_equal_p (v->same->add_val, v->add_val)
! #endif
! 		       )
  		{
  		  v->dest_reg = v->same->dest_reg;
  		  v->shared = 1;
--- 2849,2855 ----
  	      else if (unroll_type != UNROLL_COMPLETELY
  		       && v->giv_type == DEST_ADDR
  		       && v->same && v->same->giv_type == DEST_ADDR
! 		       && v->same->unrolled)
  		{
  		  v->dest_reg = v->same->dest_reg;
  		  v->shared = 1;

Re: One bug in unroll.c

[Jim Wilson]

	 I think if the current aliase code is released in
	egcs as is, the egcs project may start on a wrong foot. To the
	least, the first public release of egcs should not be worse
	than gcc2 ss-970802, which is egcs based upon.

The whole point of this project is to add new features and optimizations to
gcc.  There will of course be problems when new features are added in.

The intial list of new features (including the aliasing code) has certainly
caused problems, and we are working on fixing them as fast as we can.  It has
only been 7 days since the project started, though.  Give us some more time
to fix the bugs, and egcs will be as good as the gcc2 development sources
that we started with.

We will of course make a public release only when egcs is stable enough
for general use.

We will then add in some more features/optimizations and start all over
again with the bug fixing.

Jim