GCC 4.1 Status Report (2006-02-14)

GCC 4.1 Status Report (2006-02-14)

[Mark Mitchell]

In the last few days, many of the key obstacles to a 4.1 release were
removed, including, but not limited to:

1) The -mlong-double-128 patches have gone in.
2) Jason fixed some RVO issues.
3) Michael fixed the zero-width bitfield vs. #pragma pack issue.

Unfortunately, there are still two PRs for regressions that I think
merit P1:

PR26258: wrong code caused by incorrect alias analyis.

PR26209: an ICE-on-valid for simple C++ code, from Boost, using a
pointer-to-member.

We could argue about PR26209, but I thin PR26258 is too embarrassing to
have in a release.  Fortunately, a patch has been posted for PR26258,
and Richard G. claims to have a patch for PR26209.

It's certainly time to get GCC 4.1 out the door.  Therefore, at 12:01 AM
GMT-8 on Wednesday, February 15th, the 4.1 branch will be frozen.  All
non-documentation patches after that point will require my explicit
approval; please attach patches to PRs, and Cc: to me.  Hopefully, the
two P1s will be fixed tomorrow, and I can make a release candidate as
early as Wednesday evening.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: GCC 4.1 Status Report (2006-02-14)

[Richard Guenther]

On 2/14/06, Mark Mitchell <mark@codesourcery.com> wrote:
> In the last few days, many of the key obstacles to a 4.1 release were
> removed, including, but not limited to:
>
> 1) The -mlong-double-128 patches have gone in.
> 2) Jason fixed some RVO issues.
> 3) Michael fixed the zero-width bitfield vs. #pragma pack issue.
>
> Unfortunately, there are still two PRs for regressions that I think
> merit P1:
>
> PR26258: wrong code caused by incorrect alias analyis.

This is now fixed on both the branch and the mainline.

> PR26209: an ICE-on-valid for simple C++ code, from Boost, using a
> pointer-to-member.
>
> We could argue about PR26209, but I thin PR26258 is too embarrassing to
> have in a release.  Fortunately, a patch has been posted for PR26258,
> and Richard G. claims to have a patch for PR26209.

I guess you meant 26258, the patch for 26029 is by Zdenek and still
lacks a review:
http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00933.html

Richard.

Re: GCC 4.1 Status Report (2006-02-14)

[Mark Mitchell]

Richard Guenther wrote:

>>PR26258: wrong code caused by incorrect alias analyis.
> 
> This is now fixed on both the branch and the mainline.

Good.

> I guess you meant 26258, the patch for 26029 is by Zdenek and still
> lacks a review:
> http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00933.html

I see Diego has now reviewed it.

We'll proceed with the freeze as previously announced, at midnight,
tonight, here in California.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[H. J. Lu]

On Tue, Feb 14, 2006 at 11:48:36AM -0800, Mark Mitchell wrote:
> Richard Guenther wrote:
> 
> >>PR26258: wrong code caused by incorrect alias analyis.
> > 
> > This is now fixed on both the branch and the mainline.
> 
> Good.
> 
> > I guess you meant 26258, the patch for 26029 is by Zdenek and still
> > lacks a review:
> > http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00933.html
> 
> I see Diego has now reviewed it.
> 
> We'll proceed with the freeze as previously announced, at midnight,
> tonight, here in California.
> 

I am senting this on behalf of Denis. We have tested it on ia64, x86
and x86-64. There are no regressions. It may affect many targets:

arc/arc.h:#define HAVE_PRE_INCREMENT 1
arc/arc.h:#define HAVE_PRE_DECREMENT 1
arm/arm.h:#define HAVE_POST_INCREMENT   1
arm/arm.h:#define HAVE_PRE_INCREMENT    TARGET_ARM
arm/arm.h:#define HAVE_POST_DECREMENT   TARGET_ARM
arm/arm.h:#define HAVE_PRE_DECREMENT    TARGET_ARM
avr/avr.h:#define HAVE_POST_INCREMENT 1
avr/avr.h:#define HAVE_PRE_DECREMENT 1
bfin/bfin.h:#define HAVE_POST_INCREMENT 1
bfin/bfin.h:#define HAVE_POST_DECREMENT 1
bfin/bfin.h:#define HAVE_PRE_DECREMENT  1
c4x/c4x.h:#define HAVE_POST_INCREMENT 1
c4x/c4x.h:#define HAVE_PRE_INCREMENT 1
c4x/c4x.h:#define HAVE_POST_DECREMENT 1
c4x/c4x.h:#define HAVE_PRE_DECREMENT 1
cris/cris.h:#define HAVE_POST_INCREMENT 1
crx/crx.h:#define HAVE_POST_INCREMENT  1
crx/crx.h:#define HAVE_POST_DECREMENT  1
h8300/h8300.h:#define HAVE_POST_INCREMENT 1
h8300/h8300.h:#define HAVE_PRE_DECREMENT 1
h8300/h8300.h:#define HAVE_POST_DECREMENT TARGET_H8300SX
h8300/h8300.h:#define HAVE_PRE_INCREMENT TARGET_H8300SX
ia64/ia64.h:#define HAVE_POST_INCREMENT 1
ia64/ia64.h:#define HAVE_POST_DECREMENT 1
m32c/m32c.h:#define HAVE_PRE_DECREMENT 1
m32c/m32c.h:#define HAVE_POST_INCREMENT 1
m32r/m32r.h:#define HAVE_PRE_DECREMENT  1
m32r/m32r.h:#define HAVE_PRE_INCREMENT  1
m32r/m32r.h:#define HAVE_POST_INCREMENT 1
m68hc11/m68hc11.h:#define HAVE_POST_INCREMENT (TARGET_M6812 &&
TARGET_AUTO_INC_DEC)
m68hc11/m68hc11.h:#define HAVE_PRE_INCREMENT  (TARGET_M6812 &&
TARGET_AUTO_INC_DEC)
m68hc11/m68hc11.h:#define HAVE_POST_DECREMENT (TARGET_M6812 &&
TARGET_AUTO_INC_DEC)
m68hc11/m68hc11.h:#define HAVE_PRE_DECREMENT  (TARGET_M6812 &&
TARGET_AUTO_INC_DEC)
m68k/m68k.h:#define HAVE_POST_INCREMENT 1
m68k/m68k.h:#define HAVE_PRE_DECREMENT 1
mn10300/mn10300.h:#define HAVE_POST_INCREMENT (TARGET_AM33)
pa/pa.h:#define HAVE_POST_INCREMENT (pa_cpu < PROCESSOR_8000)
pa/pa.h:#define HAVE_POST_DECREMENT (pa_cpu < PROCESSOR_8000)
pa/pa.h:#define HAVE_PRE_DECREMENT (pa_cpu < PROCESSOR_8000)
pa/pa.h:#define HAVE_PRE_INCREMENT (pa_cpu < PROCESSOR_8000)
pdp11/pdp11.h:#define HAVE_POST_INCREMENT 1
pdp11/pdp11.h:#define HAVE_PRE_DECREMENT 1
rs6000/rs6000.h:#define HAVE_PRE_DECREMENT 1
rs6000/rs6000.h:#define HAVE_PRE_INCREMENT 1
sh/sh.h:#define HAVE_POST_INCREMENT  TARGET_SH1
sh/sh.h:#define HAVE_PRE_DECREMENT   TARGET_SH1
stormy16/stormy16.h:#define HAVE_POST_INCREMENT 1
stormy16/stormy16.h:#define HAVE_PRE_DECREMENT 1
vax/vax.h:#define HAVE_POST_INCREMENT 1
vax/vax.h:#define HAVE_PRE_DECREMENT 1


H.J.
----
2006-02-13  Denis Nagorny <denis_nagorny@linux.intel.com>

        PR rtl-optimization/25603
        * reload.c (reg_inc_found_and_valid_p): New.
	(regno_clobbered_p): Handle REG_INC as 25603 workaround.

Index: reload.c
===================================================================
*** reload.c	(revision 110738)
--- reload.c	(working copy)
*************** static int find_inc_amount (rtx, rtx);
*** 281,286 ****
--- 281,287 ----
  static int refers_to_mem_for_reload_p (rtx);
  static int refers_to_regno_for_reload_p (unsigned int, unsigned int,
  					 rtx, rtx *);
+ static int reg_inc_found_and_valid_p(unsigned int, unsigned int, rtx);
  \f
  /* Determine if any secondary reloads are needed for loading (if IN_P is
     nonzero) or storing (if IN_P is zero) X to or from a reload register of
*************** find_inc_amount (rtx x, rtx inced)
*** 6941,6946 ****
--- 6942,6975 ----
    return 0;
  }
  \f
+ /* Return 1 if insn uses the register with REG_INC note and with REGNO  greater
+    or equal then regno. Created as part of workaround for 25603 */
+ 
+ static int 
+ reg_inc_found_and_valid_p(unsigned int regno ATTRIBUTE_UNUSED,
+                           unsigned int endregno ATTRIBUTE_UNUSED,
+                           rtx insn ATTRIBUTE_UNUSED)
+ {
+ #ifdef AUTO_INC_DEC
+   rtx link;
+ 
+   gcc_assert (insn);
+ 
+   if (! INSN_P (insn))
+     return 0;
+     
+   for (link = REG_NOTES (insn); link; link = XEXP (link, 1))
+     if (REG_NOTE_KIND (link) == REG_INC)
+       {
+         unsigned int test = (int) REGNO (XEXP (link, 0)); 
+         if (test >= regno && test < endregno)
+ 	  return 1; 
+       }
+ #endif 
+   return 0;
+ 
+ }
+ 
  /* Return 1 if register REGNO is the subject of a clobber in insn INSN.
     If SETS is nonzero, also consider SETs.  REGNO must refer to a hard
     register.  */
*************** regno_clobbered_p (unsigned int regno, r
*** 6966,6971 ****
--- 6995,7003 ----
        return test >= regno && test < endregno;
      }
  
+   if (reg_inc_found_and_valid_p(regno, endregno, insn))
+     return 1; 
+   
    if (GET_CODE (PATTERN (insn)) == PARALLEL)
      {
        int i = XVECLEN (PATTERN (insn), 0) - 1;
*************** regno_clobbered_p (unsigned int regno, r
*** 6982,6987 ****
--- 7014,7021 ----
  	      if (test >= regno && test < endregno)
  		return 1;
  	    }
+           if (reg_inc_found_and_valid_p(regno, endregno, elt))
+             return 1; 
  	}
      }
  

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[Jim Wilson]

H. J. Lu wrote:
>         PR rtl-optimization/25603
>         * reload.c (reg_inc_found_and_valid_p): New.
> 	(regno_clobbered_p): Handle REG_INC as 25603 workaround.

I don't believe this is safe.  If you look at the uses of 
regno_clobbered_p in reload.c, the comments clearly indicate that we are 
looking for registers used in clobbers.  So unconditionally adding code 
that handles REG_INC notes will break these uses.  You have to add the 
REG_INC support the same way that the sets support was added, by adding 
another argument (or reusing the sets argument), and then modifying the 
one place we know is broken (choose_reload_regs) to use the new argument 
(or new sets argument value).
-- 
Jim Wilson, GNU Tools Support, http://www.specifix.com

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[H. J. Lu]

On Tue, Feb 14, 2006 at 01:59:21PM -0800, Jim Wilson wrote:
> H. J. Lu wrote:
> >        PR rtl-optimization/25603
> >        * reload.c (reg_inc_found_and_valid_p): New.
> >	(regno_clobbered_p): Handle REG_INC as 25603 workaround.
> 
> I don't believe this is safe.  If you look at the uses of 
> regno_clobbered_p in reload.c, the comments clearly indicate that we are 
> looking for registers used in clobbers.  So unconditionally adding code 
> that handles REG_INC notes will break these uses.  You have to add the 
> REG_INC support the same way that the sets support was added, by adding 
> another argument (or reusing the sets argument), and then modifying the 
> one place we know is broken (choose_reload_regs) to use the new argument 
> (or new sets argument value).

I see. We will add a new argument to regno_clobbered_p. 


H.J.

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[Denis Nagorny]

Jim Wilson wrote:
> I don't believe this is safe.  If you look at the uses of 
> regno_clobbered_p in reload.c, the comments clearly indicate that we are 
> looking for registers used in clobbers.  So unconditionally adding code 
> that handles REG_INC notes will break these uses.  You have to add the 
> REG_INC support the same way that the sets support was added, by adding 
> another argument (or reusing the sets argument), and then modifying the 
> one place we know is broken (choose_reload_regs) to use the new argument 
> (or new sets argument value).
Jim,
Did I understand your idea correctly? Can you comment new patch version.
It isn't fully tested but am I going in right direction?

2006-02-13  Denis Nagorny <denis_nagorny@linux.intel.com>

         PR rtl-optimization/25603
         * reload.c (reg_inc_found_and_valid_p): New. Check REG_INC note.
         (regno_clobbered_p): Use it. Reusing SETS argument for REG_INC case.
         * reload1.c (choose_reload_regs): Added call of egno_clobbered_p
         with new meaning of SETS.

Index: reload.c
===================================================================
*** reload.c    (revision 110905)
--- reload.c    (working copy)
*************** static int find_inc_amount (rtx, rtx);
*** 281,286 ****
--- 281,287 ----
   static int refers_to_mem_for_reload_p (rtx);
   static int refers_to_regno_for_reload_p (unsigned int, unsigned int,
                      rtx, rtx *);
+ static int reg_inc_found_and_valid_p(unsigned int, unsigned int, rtx);

   /* Determine if any secondary reloads are needed for loading (if IN_P is
      nonzero) or storing (if IN_P is zero) X to or from a reload register of
*************** find_inc_amount (rtx x, rtx inced)
*** 6941,6949 ****
     return 0;
   }

   /* Return 1 if register REGNO is the subject of a clobber in insn INSN.
!    If SETS is nonzero, also consider SETs.  REGNO must refer to a hard
!    register.  */

   int
   regno_clobbered_p (unsigned int regno, rtx insn, enum machine_mode mode,
--- 6942,6978 ----
     return 0;
   }

+ /* Return 1 if registers from REGNO to ENDREGNO are the subjects of a
+    REG_INC note in insn INSN.  REGNO must refer to a hard register.  */
+
+ static int
+ reg_inc_found_and_valid_p(unsigned int regno ATTRIBUTE_UNUSED,
+                           unsigned int endregno ATTRIBUTE_UNUSED,
+                           rtx insn ATTRIBUTE_UNUSED)
+ {
+ #ifdef AUTO_INC_DEC
+   rtx link;
+
+   gcc_assert (insn);
+
+   if (! INSN_P (insn))
+     return 0;
+
+   for (link = REG_NOTES (insn); link; link = XEXP (link, 1))
+     if (REG_NOTE_KIND (link) == REG_INC)
+       {
+         unsigned int test = (int) REGNO (XEXP (link, 0));
+         if (test >= regno && test < endregno)
+     return 1;
+       }
+ #endif
+   return 0;
+
+ }
+
   /* Return 1 if register REGNO is the subject of a clobber in insn INSN.
!    If SETS is 1, also consider SETs. If SETS is 2, enable checking REG_INC.
!    REGNO must refer to a hard register.  */

   int
   regno_clobbered_p (unsigned int regno, rtx insn, enum machine_mode mode,
*************** regno_clobbered_p (unsigned int regno, r
*** 6958,6964 ****
     endregno = regno + nregs;

     if ((GET_CODE (PATTERN (insn)) == CLOBBER
!        || (sets && GET_CODE (PATTERN (insn)) == SET))
         && REG_P (XEXP (PATTERN (insn), 0)))
       {
         unsigned int test = REGNO (XEXP (PATTERN (insn), 0));
--- 6987,6993 ----
     endregno = regno + nregs;

     if ((GET_CODE (PATTERN (insn)) == CLOBBER
!        || (sets == 1 && GET_CODE (PATTERN (insn)) == SET))
         && REG_P (XEXP (PATTERN (insn), 0)))
       {
         unsigned int test = REGNO (XEXP (PATTERN (insn), 0));
*************** regno_clobbered_p (unsigned int regno, r
*** 6966,6971 ****
--- 6995,7003 ----
         return test >= regno && test < endregno;
       }

+   if (sets == 2 && reg_inc_found_and_valid_p(regno, endregno, insn))
+     return 1;
+
     if (GET_CODE (PATTERN (insn)) == PARALLEL)
       {
         int i = XVECLEN (PATTERN (insn), 0) - 1;
*************** regno_clobbered_p (unsigned int regno, r
*** 6974,6980 ****
     {
       rtx elt = XVECEXP (PATTERN (insn), 0, i);
       if ((GET_CODE (elt) == CLOBBER
!          || (sets && GET_CODE (PATTERN (insn)) == SET))
           && REG_P (XEXP (elt, 0)))
         {
           unsigned int test = REGNO (XEXP (elt, 0));
--- 7006,7012 ----
     {
       rtx elt = XVECEXP (PATTERN (insn), 0, i);
       if ((GET_CODE (elt) == CLOBBER
!          || (sets == 1 && GET_CODE (PATTERN (insn)) == SET))
           && REG_P (XEXP (elt, 0)))
         {
           unsigned int test = REGNO (XEXP (elt, 0));
*************** regno_clobbered_p (unsigned int regno, r
*** 6982,6987 ****
--- 7014,7021 ----
           if (test >= regno && test < endregno)
         return 1;
         }
+           if (sets == 2 && reg_inc_found_and_valid_p(regno, endregno, elt))
+             return 1;
     }
       }

Index: reload1.c
===================================================================
*** reload1.c   (revision 110905)
--- reload1.c   (working copy)
*************** choose_reload_regs (struct insn_chain *c
*** 5780,5786 ****

           if (equiv != 0)
         {
!         if (regno_clobbered_p (regno, insn, rld[r].mode, 0))
             switch (rld[r].when_needed)
               {
               case RELOAD_FOR_OTHER_ADDRESS:
--- 5780,5787 ----

           if (equiv != 0)
         {
!         if (regno_clobbered_p (regno, insn, rld[r].mode, 0)
!                       || regno_clobbered_p (regno, insn, rld[r].mode, 2))
             switch (rld[r].when_needed)
               {
               case RELOAD_FOR_OTHER_ADDRESS:

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[James E Wilson]

On Wed, 2006-02-15 at 05:52, Denis Nagorny wrote:
> Did I understand your idea correctly? Can you comment new patch version.
> It isn't fully tested but am I going in right direction?

Yes, that is what I was suggesting.

In choose_reload_regs, you only need one regno_clobbered_p call, with
sets == 2, as that will check for both clobbers and REG_INC notes.

There are some indentation problems with the patch.  It looks like maybe
you have inconsistent usage of spaces/tabs for indentation, and/or that
your mailer converted the tabs to spaces.  You probably should put
patches in a MIME attachment.

In both the reg_inc_found_and_valid_p declaration and definition, there
should be a space before the open parenthesis.  Also, the two calls in
regno_clobbered_p have the same problem.

At the end of the reg_inc_found_and_valid_p, there is a spurious blank
line.
-- 
Jim Wilson, GNU Tools Support, http://www.specifix.com

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[Denis Nagorny]

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

James E Wilson wrote:
> Yes, that is what I was suggesting.

It's corrected and tested on ia64 and x86-64. I've attached new version.
Denis.


[-- Attachment #2: 25603.patch --]
[-- Type: text/plain, Size: 4436 bytes --]

Index: reload.c
===================================================================
*** reload.c	(revision 111135)
--- reload.c	(working copy)
*************** static int find_inc_amount (rtx, rtx);
*** 281,286 ****
--- 281,287 ----
  static int refers_to_mem_for_reload_p (rtx);
  static int refers_to_regno_for_reload_p (unsigned int, unsigned int,
  					 rtx, rtx *);
+ static int reg_inc_found_and_valid_p (unsigned int, unsigned int, rtx);
  \f
  /* Determine if any secondary reloads are needed for loading (if IN_P is
     nonzero) or storing (if IN_P is zero) X to or from a reload register of
*************** find_inc_amount (rtx x, rtx inced)
*** 6941,6949 ****
    return 0;
  }
  \f
  /* Return 1 if register REGNO is the subject of a clobber in insn INSN.
!    If SETS is nonzero, also consider SETs.  REGNO must refer to a hard
!    register.  */
  
  int
  regno_clobbered_p (unsigned int regno, rtx insn, enum machine_mode mode,
--- 6942,6977 ----
    return 0;
  }
  \f
+ /* Return 1 if registers from REGNO to ENDREGNO are the subjects of a
+    REG_INC note in insn INSN.  REGNO must refer to a hard register.  */
+ 
+ static int 
+ reg_inc_found_and_valid_p (unsigned int regno ATTRIBUTE_UNUSED,
+                            unsigned int endregno ATTRIBUTE_UNUSED,
+                            rtx insn ATTRIBUTE_UNUSED)
+ {
+ #ifdef AUTO_INC_DEC
+   rtx link;
+ 
+   gcc_assert (insn);
+ 
+   if (! INSN_P (insn))
+     return 0;
+     
+   for (link = REG_NOTES (insn); link; link = XEXP (link, 1))
+     if (REG_NOTE_KIND (link) == REG_INC)
+       {
+         unsigned int test = (int) REGNO (XEXP (link, 0)); 
+         if (test >= regno && test < endregno)
+ 	        return 1; 
+       }
+ #endif 
+   return 0;
+ }
+ 
  /* Return 1 if register REGNO is the subject of a clobber in insn INSN.
!    If SETS is 1, also consider SETs. If SETS is 2, enable checking REG_INC.
!    REGNO must refer to a hard register.  */
  
  int
  regno_clobbered_p (unsigned int regno, rtx insn, enum machine_mode mode,
*************** regno_clobbered_p (unsigned int regno, r
*** 6958,6964 ****
    endregno = regno + nregs;
  
    if ((GET_CODE (PATTERN (insn)) == CLOBBER
!        || (sets && GET_CODE (PATTERN (insn)) == SET))
        && REG_P (XEXP (PATTERN (insn), 0)))
      {
        unsigned int test = REGNO (XEXP (PATTERN (insn), 0));
--- 6986,6992 ----
    endregno = regno + nregs;
  
    if ((GET_CODE (PATTERN (insn)) == CLOBBER
!        || (sets == 1 && GET_CODE (PATTERN (insn)) == SET))
        && REG_P (XEXP (PATTERN (insn), 0)))
      {
        unsigned int test = REGNO (XEXP (PATTERN (insn), 0));
*************** regno_clobbered_p (unsigned int regno, r
*** 6966,6971 ****
--- 6994,7002 ----
        return test >= regno && test < endregno;
      }
  
+   if (sets == 2 && reg_inc_found_and_valid_p (regno, endregno, insn))
+     return 1; 
+   
    if (GET_CODE (PATTERN (insn)) == PARALLEL)
      {
        int i = XVECLEN (PATTERN (insn), 0) - 1;
*************** regno_clobbered_p (unsigned int regno, r
*** 6974,6980 ****
  	{
  	  rtx elt = XVECEXP (PATTERN (insn), 0, i);
  	  if ((GET_CODE (elt) == CLOBBER
! 	       || (sets && GET_CODE (PATTERN (insn)) == SET))
  	      && REG_P (XEXP (elt, 0)))
  	    {
  	      unsigned int test = REGNO (XEXP (elt, 0));
--- 7005,7011 ----
  	{
  	  rtx elt = XVECEXP (PATTERN (insn), 0, i);
  	  if ((GET_CODE (elt) == CLOBBER
! 	       || (sets == 1 && GET_CODE (PATTERN (insn)) == SET))
  	      && REG_P (XEXP (elt, 0)))
  	    {
  	      unsigned int test = REGNO (XEXP (elt, 0));
*************** regno_clobbered_p (unsigned int regno, r
*** 6982,6987 ****
--- 7013,7020 ----
  	      if (test >= regno && test < endregno)
  		return 1;
  	    }
+ 	  if (sets == 2 && reg_inc_found_and_valid_p (regno, endregno, elt))
+ 	    return 1; 
  	}
      }
  
Index: reload1.c
===================================================================
*** reload1.c	(revision 110905)
--- reload1.c	(working copy)
*************** choose_reload_regs (struct insn_chain *c
*** 5780,5786 ****
  
  	      if (equiv != 0)
  		{
! 		  if (regno_clobbered_p (regno, insn, rld[r].mode, 0))
  		    switch (rld[r].when_needed)
  		      {
  		      case RELOAD_FOR_OTHER_ADDRESS:
--- 5780,5786 ----
  
  	      if (equiv != 0)
  		{
! 		  if (regno_clobbered_p (regno, insn, rld[r].mode, 2))
  		    switch (rld[r].when_needed)
  		      {
  		      case RELOAD_FOR_OTHER_ADDRESS:

[-- Attachment #3: ChangeLog --]
[-- Type: text/plain, Size: 317 bytes --]

2006-02-16  Denis Nagorny <denis_nagorny@linux.intel.com>

	PR rtl-optimization/25603
	* reload.c (reg_inc_found_and_valid_p): New. Check REG_INC note.
	(regno_clobbered_p): Use it. Reusing SETS argument for REG_INC case.
	* reload1.c (choose_reload_regs): Added call of regno_clobbered_p 
	with new meaning of SETS.

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[Andrew Pinski]

On Feb 16, 2006, at 2:59 PM, Denis Nagorny wrote:

> James E Wilson wrote:
>> Yes, that is what I was suggesting.
>
> It's corrected and tested on ia64 and x86-64. I've attached new 
> version.
> Denis.
> + /* Return 1 if registers from REGNO to ENDREGNO are the subjects of a
> +    REG_INC note in insn INSN.  REGNO must refer to a hard register.  
> */
> +
> + static int
> + reg_inc_found_and_valid_p (unsigned int regno ATTRIBUTE_UNUSED,
> +                            unsigned int endregno ATTRIBUTE_UNUSED,
> +                            rtx insn ATTRIBUTE_UNUSED)
> + {
> + #ifdef AUTO_INC_DEC


What about defining the code this way:
#ifdef AUTO_INC_DEC
+ static int
+ reg_inc_found_and_valid_p (unsigned int regno,
+                            unsigned int endregno,
+                            rtx insn)
+ {

...
}
#else

#define reg_inc_found_and_valid_p(regno,endregno,insn) 0

#endif

So you can remove ugly ATTRIBUTE_UNUSED.

Thanks,
Andrew Pinski

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[H. J. Lu]

On Thu, Feb 16, 2006 at 03:03:02PM -0500, Andrew Pinski wrote:
> 
> On Feb 16, 2006, at 2:59 PM, Denis Nagorny wrote:
> 
> >James E Wilson wrote:
> >>Yes, that is what I was suggesting.
> >
> >It's corrected and tested on ia64 and x86-64. I've attached new 
> >version.
> >Denis.
> >+ /* Return 1 if registers from REGNO to ENDREGNO are the subjects of a
> >+    REG_INC note in insn INSN.  REGNO must refer to a hard register.  
> >*/
> >+
> >+ static int
> >+ reg_inc_found_and_valid_p (unsigned int regno ATTRIBUTE_UNUSED,
> >+                            unsigned int endregno ATTRIBUTE_UNUSED,
> >+                            rtx insn ATTRIBUTE_UNUSED)
> >+ {
> >+ #ifdef AUTO_INC_DEC
> 
> 
> What about defining the code this way:
> #ifdef AUTO_INC_DEC
> + static int
> + reg_inc_found_and_valid_p (unsigned int regno,
> +                            unsigned int endregno,
> +                            rtx insn)
> + {
> 
> ...
> }
> #else
> 
> #define reg_inc_found_and_valid_p(regno,endregno,insn) 0
> 
> #endif
> 
> So you can remove ugly ATTRIBUTE_UNUSED.

Should

+static int reg_inc_found_and_valid_p (unsigned int, unsigned int, rtx);

be removed if we do this way?


H.J.

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[Andrew Pinski]

On Feb 16, 2006, at 3:20 PM, H. J. Lu wrote:

>
> Should
>
> +static int reg_inc_found_and_valid_p (unsigned int, unsigned int, 
> rtx);
>
> be removed if we do this way?

Yes, in fact it does not need to be there in the first place as
the function is declared before the use.

-- Pinski

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[Denis Nagorny]

Andrew Pinski wrote:

> What about defining the code this way:
> #ifdef AUTO_INC_DEC
> + static int
> + reg_inc_found_and_valid_p (unsigned int regno,
> +                            unsigned int endregno,
> +                            rtx insn)
> + {
> 
> ...
> }
> #else
> 
> #define reg_inc_found_and_valid_p(regno,endregno,insn) 0
> 
> #endif
> 
> So you can remove ugly ATTRIBUTE_UNUSED.
> 
> Thanks,
> Andrew Pinski
> 

Yes. It will be more ellegant and easier for compiler to remove following conditions, but
what about
+ static int reg_inc_found_and_valid_p (unsigned int, unsigned int, rtx);
In this case I must hide this under preprocessor too. I feared to overburden code by preprocessor
expressions.
What is you opinion?

Denis.

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[Denis Nagorny]

Andrew Pinski wrote:
> 
> On Feb 16, 2006, at 3:20 PM, H. J. Lu wrote:
> 
>>
>> Should
>>
>> +static int reg_inc_found_and_valid_p (unsigned int, unsigned int, rtx);
>>
>> be removed if we do this way?
> 
> 
> Yes, in fact it does not need to be there in the first place as
> the function is declared before the use.
> 
> -- Pinski
> 

I'll do it.

Denis

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[James E Wilson]

On Thu, 2006-02-16 at 11:59, Denis Nagorny wrote:
> It's corrected and tested on ia64 and x86-64. I've attached new version.
> Denis.

This look pretty good.  There is still one place where the spacing looks
funny.

> +         if (test >= regno && test < endregno)
> + 	        return 1; 

Checking this in emacs, I see that the if statement is preceded by 8
spaces, and the return is preceded by a tab and 8 spaces.  It should be
only a tab before the if, and a tab and 2 spaces before the return.  Do
you have tab stops set every 8 characters?  It looks like you might have
tabs set to 2 characters, otherwise I don't see how you could make this
mistake and not notice it.

There are also 3 other lines that start with spaces that should start
with tabs, though this is only a minor problem.

Andrew Pinksi made a good suggestion.  The AUTO_INC_DEC ifdefs should be
placed around the reg_inc_found_and_valid_p function instead of inside
it.  This lets you get rid of the ATTRIBUTE_UNUSED stuff, and it also is
more efficient, since it saves the cost of a call for !AUTO_INC_DEC
targets.
-- 
Jim Wilson, GNU Tools Support, http://www.specifix.com

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[H. J. Lu]

On Thu, Feb 16, 2006 at 12:34:19PM -0800, James E Wilson wrote:
> On Thu, 2006-02-16 at 11:59, Denis Nagorny wrote:
> > It's corrected and tested on ia64 and x86-64. I've attached new version.
> > Denis.
> 
> This look pretty good.  There is still one place where the spacing looks
> funny.
> 
> > +         if (test >= regno && test < endregno)
> > + 	        return 1; 
> 
> Checking this in emacs, I see that the if statement is preceded by 8
> spaces, and the return is preceded by a tab and 8 spaces.  It should be
> only a tab before the if, and a tab and 2 spaces before the return.  Do
> you have tab stops set every 8 characters?  It looks like you might have
> tabs set to 2 characters, otherwise I don't see how you could make this
> mistake and not notice it.
> 
> There are also 3 other lines that start with spaces that should start
> with tabs, though this is only a minor problem.
> 
> Andrew Pinksi made a good suggestion.  The AUTO_INC_DEC ifdefs should be
> placed around the reg_inc_found_and_valid_p function instead of inside
> it.  This lets you get rid of the ATTRIBUTE_UNUSED stuff, and it also is
> more efficient, since it saves the cost of a call for !AUTO_INC_DEC
> targets.

I took the liberty to fix the format issue on behalf of Denis. Is this
OK for mainline?

Thanks.


H.J.
----
2006-02-16  Denis Nagorny <denis_nagorny@linux.intel.com>

	PR rtl-optimization/25603
	* reload.c (reg_inc_found_and_valid_p): New. Check REG_INC note.
	(regno_clobbered_p): Use it. Reusing SETS argument for REG_INC case.
	* reload1.c (choose_reload_regs): Added call of regno_clobbered_p 
	with new meaning of SETS.

--- gcc/reload.c.reg_inc	2006-02-14 09:37:12.000000000 -0800
+++ gcc/reload.c	2006-02-16 14:40:02.000000000 -0800
@@ -6941,9 +6941,39 @@ find_inc_amount (rtx x, rtx inced)
   return 0;
 }
 \f
+/* Return 1 if registers from REGNO to ENDREGNO are the subjects of a
+   REG_INC note in insn INSN.  REGNO must refer to a hard register.  */
+
+#ifdef AUTO_INC_DEC
+static int 
+reg_inc_found_and_valid_p (unsigned int regno, unsigned int endregno,
+			   rtx insn)
+{
+  rtx link;
+
+  gcc_assert (insn);
+
+  if (! INSN_P (insn))
+    return 0;
+    
+  for (link = REG_NOTES (insn); link; link = XEXP (link, 1))
+    if (REG_NOTE_KIND (link) == REG_INC)
+      {
+	unsigned int test = (int) REGNO (XEXP (link, 0));
+	if (test >= regno && test < endregno)
+	  return 1; 
+      }
+  return 0;
+}
+#else
+
+#define reg_inc_found_and_valid_p(regno,endregno,insn) 0
+
+#endif 
+
 /* Return 1 if register REGNO is the subject of a clobber in insn INSN.
-   If SETS is nonzero, also consider SETs.  REGNO must refer to a hard
-   register.  */
+   If SETS is 1, also consider SETs.  If SETS is 2, enable checking
+   REG_INC.  REGNO must refer to a hard register.  */
 
 int
 regno_clobbered_p (unsigned int regno, rtx insn, enum machine_mode mode,
@@ -6958,7 +6988,7 @@ regno_clobbered_p (unsigned int regno, r
   endregno = regno + nregs;
 
   if ((GET_CODE (PATTERN (insn)) == CLOBBER
-       || (sets && GET_CODE (PATTERN (insn)) == SET))
+       || (sets == 1 && GET_CODE (PATTERN (insn)) == SET))
       && REG_P (XEXP (PATTERN (insn), 0)))
     {
       unsigned int test = REGNO (XEXP (PATTERN (insn), 0));
@@ -6966,6 +6996,9 @@ regno_clobbered_p (unsigned int regno, r
       return test >= regno && test < endregno;
     }
 
+  if (sets == 2 && reg_inc_found_and_valid_p (regno, endregno, insn))
+    return 1; 
+  
   if (GET_CODE (PATTERN (insn)) == PARALLEL)
     {
       int i = XVECLEN (PATTERN (insn), 0) - 1;
@@ -6974,7 +7007,7 @@ regno_clobbered_p (unsigned int regno, r
 	{
 	  rtx elt = XVECEXP (PATTERN (insn), 0, i);
 	  if ((GET_CODE (elt) == CLOBBER
-	       || (sets && GET_CODE (PATTERN (insn)) == SET))
+	       || (sets == 1 && GET_CODE (PATTERN (insn)) == SET))
 	      && REG_P (XEXP (elt, 0)))
 	    {
 	      unsigned int test = REGNO (XEXP (elt, 0));
@@ -6982,6 +7015,9 @@ regno_clobbered_p (unsigned int regno, r
 	      if (test >= regno && test < endregno)
 		return 1;
 	    }
+	  if (sets == 2
+	      && reg_inc_found_and_valid_p (regno, endregno, elt))
+	    return 1; 
 	}
     }
 
--- gcc/reload1.c.reg_inc	2006-02-16 06:47:16.000000000 -0800
+++ gcc/reload1.c	2006-02-16 14:35:37.000000000 -0800
@@ -5780,7 +5780,7 @@ choose_reload_regs (struct insn_chain *c
 
 	      if (equiv != 0)
 		{
-		  if (regno_clobbered_p (regno, insn, rld[r].mode, 0))
+		  if (regno_clobbered_p (regno, insn, rld[r].mode, 2))
 		    switch (rld[r].when_needed)
 		      {
 		      case RELOAD_FOR_OTHER_ADDRESS:

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[James E Wilson]

On Thu, 2006-02-16 at 14:46, H. J. Lu wrote:
> I took the liberty to fix the format issue on behalf of Denis. Is this
> OK for mainline?

Yes, this looks good to me.
-- 
Jim Wilson, GNU Tools Support, http://www.specifix.com

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[H. J. Lu]

On Thu, Feb 16, 2006 at 03:46:57PM -0800, James E Wilson wrote:
> On Thu, 2006-02-16 at 14:46, H. J. Lu wrote:
> > I took the liberty to fix the format issue on behalf of Denis. Is this
> > OK for mainline?
> 
> Yes, this looks good to me.

I checked it in. Here is the testcase.


H.J.
----
2006-02-16  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/25603
	* gfortran.dg/pr25603.f: New testcase.

--- gcc/testsuite/gfortran.dg/pr25603.f.25603	2006-02-16 17:42:01.000000000 -0800
+++ gcc/testsuite/gfortran.dg/pr25603.f	2006-02-16 20:07:54.000000000 -0800
@@ -0,0 +1,60 @@
+C { dg-do run }
+C
+C PR rtl-optimization/25603
+C Check if reload handles REG_INC notes correctly.
+      PROGRAM BAR
+      IMPLICIT REAL (A-H, O-Z)
+      DIMENSION WORK(250)
+
+      XSTART = 201.0
+
+      CALL BAR1(NX,NY,NZ,NT,NTIME,NWINDX,NWINDY,NSINKS,NFILT,
+     *XSTART,YSTART,ZSTART,TSTART,DELTAX,DELTAY,DELTAZ,DELTAT,PHI,DL,
+     *DX,DY,DZ,WORK,IB,IK,ITY,NCOUNT,NPRINT,NGRAPH,NPSTEP,NGSTEP)
+      STOP
+      END
+
+      SUBROUTINE BAR2(NX,NY,NZ,NT,NTIME,NWINDX,ISH,NSMT,NFILT,
+     * XSTART,YSTART,ZSTART,TSTART,DELTAX,DELTAY,DELTAZ,DELTAT,PHI,DL,
+     * DX,DY,DZ,IB,IK,ITY,NCOUNT,NPRINT,NGRAPH,NPSTEP,NGSTEP,LFINAL,
+     * C,STEPC,POTT,STEPT,UX,STEPU,VY,STEPV,WZ,PRES,STEPP,Q,DKZM,DKZH,
+     * ELEV,ELEVX,ELEVY,Z0,HMIX,STEPH,TAVR,OBUK,USTR,TSTR,VDEP,DEP,
+     * ZET,HVAR,UM,VM,UG,VG,TM,DKM,DCDX,DCDY,AN,BN,CN,HELP,HELPA)
+      IMPLICIT REAL (A-H, O-Z)
+
+      DIMENSION  C(*),STEPC(*),POTT(*),STEPT(*),UX(*),STEPU(*),
+     * VY(*),STEPV(*),WZ(*),PRES(*),STEPP(*),Q(*),DKZM(*),DKZH(*),
+     * ELEV(*),ELEVX(*),ELEVY(*),Z0(*),HMIX(*),STEPH(*),TAVR(*),
+     * OBUK(*),USTR(*),TSTR(*),VDEP(*), DEP(*),ZET(*),HVAR(*),
+     * UM(*),VM(*),UG(*),VG(*),TM(*),DKM(*), DCDX(*),DCDY(*),
+     * AN(*),BN(*),CN(*),HELP(*),HELPA(*)
+C
+
+      RETURN
+      END
+
+      SUBROUTINE BAR1(NX,NY,NZ,NT,NTIME,NWINDX,NWINDY,NSINKS,NFILT,
+     *XSTART,YSTART,ZSTART,TSTART,DELTAX,DELTAY,DELTAZ,DELTAT,PHI,DL,
+     *DX,DY,DZ,WORK,IB,IK,ITY,NCOUNT,NPRINT,NGRAPH,NPSTEP,NGSTEP)
+
+      IMPLICIT REAL (A-H, O-Z)
+      DIMENSION  WORK(*)
+
+      if (XSTART .NE. 201.0) then
+	call abort
+      endif
+
+      CALL BAR2(NX,NY,NZ,NT,NTIME,NWINDX,NWINDY,NSINKS,NFILT,XSTART,
+     * YSTART,ZSTART,TSTART,DELTAX,DELTAY,DELTAZ,DELTAT,PHI,DL,
+     * DX,DY,DZ,IB,IK,ITY,NCOUNT,NPRINT,NGRAPH,NPSTEP,NGSTEP,LAST,
+     * WORK(LC),WORK(LCSTEP),WORK(LPOT),WORK(LTSTEP),WORK(LUX),
+     * WORK(LUSTEP),WORK(LVY),WORK(LVSTEP),WORK(LWZ),WORK(LPI),
+     * WORK(LPSTEP),WORK(LQ),WORK(LDKZM),WORK(LDKZH),WORK(LELEV),
+     * WORK(LELEVX),WORK(LELEVY),WORK(LZ0),WORK(LHMIX),WORK(LSTEPH),
+     * WORK(LTAVR),WORK(LOBUK),WORK(LUSTR),WORK(LTSTR),WORK(LVDEP),
+     * WORK(LDEP),WORK(LZET),WORK(LHVAR),WORK(LUM),WORK(LVM),WORK(LUG),
+     * WORK(LVG),WORK(LTM),WORK(LKM),WORK(LDCDX),WORK(LDCDY),WORK(LAN),
+     * WORK(LBN),WORK(LCN),WORK(LHELP),WORK(LHELPA))
+
+      RETURN
+      END

Re: PATCH: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[Richard Guenther]

On 2/17/06, H. J. Lu <hjl@lucon.org> wrote:
> On Thu, Feb 16, 2006 at 03:46:57PM -0800, James E Wilson wrote:
> > On Thu, 2006-02-16 at 14:46, H. J. Lu wrote:
> > > I took the liberty to fix the format issue on behalf of Denis. Is this
> > > OK for mainline?
> >
> > Yes, this looks good to me.
>
> I checked it in. Here is the testcase.

Would you please check that  in also?

Thanks,
Richard.