Re: Further analysis of g77.f-torture/execute/20000503-1.f

[Toon Moene <toon at moene dot indiv dot nluug dot nl>]

Richard Henderson wrote:

> On Thu, Jul 06, 2000 at 01:22:59AM -0000, Billinghurst, David (CRTS) wrote:

> > As a guess, the compiler incorrectly tries to transformation
> >   [2*(I0+N-1)-4*I0+4]/4 => (I0+N-1)/2 - I0 + 1

> You're probably right.  You might look at what's going on
> in extract_muldiv.  That function has been notoriously buggy.

I think I've got it.  Note that in Fortran, integer division rounds to
zero (i.e., TRUNC_DIV_EXPR in the tree representation).

Analysis:

Function extract_muldiv in file fold-const.c tries to simplify integer
expressions of the form (N +/- C1) op C2, where `op' is multiplication,
division or modulus.

Obviously, for multiplication this can be transformed into N * C2 +/- C1
* C2, where the last multiplication is replaced by its result.

When `op' is division or modulus, the transformation is only correct
when C1 op C2 is exact.  However, in the PLUS_EXPR, MINUS_EXPR case of
the switch that constitutes this function, this was not explicitly
checked.

Hence the following patch (bootstrapped and make -k check'd on
alphaev6-unknown-linux-gnu):

2000-07-08  Toon Moene  <toon@moene.indiv.nluug.nl>

	* fold-const.c (extract_muldiv) case PLUS_EXPR, MINUS_EXPR:
	Check whether c divides op1 exactly if operation is not
	multiplication.

*** fold-const.c.orig   Sun Jul  2 11:06:52 2000
--- fold-const.c        Sat Jul  8 15:18:58 2000
*************** extract_muldiv (t, c, code, wide_type)
*** 4502,4509 ****
        }
  
!       /* Now do the operation and verify it doesn't overflow.  */
!       op1 = const_binop (code, convert (ctype, op1), convert (ctype,
c), 0);
!       if (op1 == 0 || TREE_OVERFLOW (op1))
!       break;
  
        /* If we have an unsigned type is not a sizetype, we cannot
widen
--- 4502,4516 ----
        }
  
!       /* If it's a multiply or a division/modulus operation of a
multiple
!          of our constant, do the operation and verify it doesn't
overflow.  */
!       if (code == MULT_EXPR
!         || integer_zerop (const_binop (TRUNC_MOD_EXPR, op1, c, 0)))
!         {
!           op1 = const_binop (code, convert (ctype, op1), convert
(ctype, c), 0);
!           if (op1 == 0 || TREE_OVERFLOW (op1))
!             break;
!         }
!       else
!         break;
  
        /* If we have an unsigned type is not a sizetype, we cannot
widen

Please review this patch - it might need some polishing (for one, I do
not understand why I need *TRUNC*_MOD_EXPR - I just copied the condition
from earlier in the function).  The comment obviously can use some
stylistic improvement.

Thanks go to David Billinghurst, for his solid detective work through
the snapshots, and Geoff Keating and Richard Henderson for helpful
hints.

All errors in the above are mine, though.

[ Diff attached because the above will probably be severly maimed
  by Netscape ]

-- 
Toon Moene - mailto:toon@moene.indiv.nluug.nl - phoneto: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
GNU Fortran 77: http://gcc.gnu.org/onlinedocs/g77_news.html
GNU Fortran 95: http://g95.sourceforge.net/ (under construction)
*** fold-const.c.orig	Sun Jul  2 11:06:52 2000
--- fold-const.c	Sat Jul  8 15:18:58 2000
*************** extract_muldiv (t, c, code, wide_type)
*** 4502,4509 ****
  	}
  
!       /* Now do the operation and verify it doesn't overflow.  */
!       op1 = const_binop (code, convert (ctype, op1), convert (ctype, c), 0);
!       if (op1 == 0 || TREE_OVERFLOW (op1))
! 	break;
  
        /* If we have an unsigned type is not a sizetype, we cannot widen
--- 4502,4516 ----
  	}
  
!       /* If it's a multiply or a division/modulus operation of a multiple
!          of our constant, do the operation and verify it doesn't overflow.  */
!       if (code == MULT_EXPR
! 	  || integer_zerop (const_binop (TRUNC_MOD_EXPR, op1, c, 0)))
!         {
!           op1 = const_binop (code, convert (ctype, op1), convert (ctype, c), 0);
!           if (op1 == 0 || TREE_OVERFLOW (op1))
!             break;
!         }
!       else
!         break;
  
        /* If we have an unsigned type is not a sizetype, we cannot widen