hpux20 pointer optimization bug

Jeffrey A Law law@cygnus.com
Sun Jun 7 01:19:00 GMT 1998


Jim -- there's an unroll patch at the bottom of this message.

  In message <199806041651.JAA25760@cygnus.com>you write:
  > This bug is seen in gcc only with the combination -O -funroll-loops. It
  > results in a SIGSEGV when run under gdb 4.17.  It goes away if any of
  > the following is done:
  > 
  > reduce level of loop nesting (apparently the pointers are trashed when
  > leaving the middle level loop)
  > 
  > use saner notation, replacing e.g. aj[i] by a[j][i].  This
  > notation was used in order to simulate the treatment of the
  > corresponding Fortran code.
  > 
  > put offending code in main()
There's a bug in unroll.c.

The net effect is the register used for memory addresses in the
inner loop isn't initialized in some cases.

  > This code has been run on a number of compilers and architectures, and
  > runs incorrectly only with gnu compilers on hppa.  It's worse in
  > gcc-0.5.23 (not restricted to triple nested or unrolled loops).
I believe the bug is a generic problem.  It may be the case that the
complex addressing modes on the PA just make it more likely to trigger
on that platform.

  > More often than not, -O code runs faster than -O2, but I have not seen
  > any numerical differences in results.  There are a few cases where
  > -funroll-loops slows code down by more than a factor of 2.  Are these
  > known HAIFA effects or a problem with hppa support?
This would be a suprise to me.  Generally -O2 code runs much faster
than -O code on the PA.  However, I rarely use -funroll-loops, so
that may be a factor in the slowdowns you are seeing.

Jim --

What appears to be happening is we have multiple address GIVS that
have been combined with the same dest_reg GIV.


Processing for the first GIV falls into this code:
                  /* If the address hasn't been checked for validity yet, do so
                     now, and fail completely if either the first or the last
                     unrolled copy of the address is not a valid address
                     for the instruction that uses it.  */
                  if (v->dest_reg == tem
                      && ! verify_addresses (v, giv_inc, unroll_number))
                    {
                      for (v2 = v->next_iv; v2; v2 = v2->next_iv)
                        if (v2->same_insn == v)
                          v2->same_insn = 0;

                      if (loop_dump_stream)
                        fprintf (loop_dump_stream,
                                 "Invalid address for giv at insn %d\n",
                                 INSN_UID (v->insn));
                      continue;
                    }


The address is not valid, so we clear any v2->same_insn fields that
point back to the first GIV, print the nice warning message and start
processing the next GIV.  Note that we do not emit code to initialize
the new register for this GIV.


The next GIV has v->same which points to the first GIV.  So we
get into this code:

              /* If multiple address GIVs have been combined with the
                 same dest_reg GIV, do not create a new register for
                 each.  */
              else if (unroll_type != UNROLL_COMPLETELY
                       && v->giv_type == DEST_ADDR
                       && v->same && v->same->giv_type == DEST_ADDR
                       && v->same->unrolled
                       /* combine_givs_p may return true for some cases
                          where the add and mult values are not equal.
                          To share a register here, the values must be
                          equal.  */
                       && rtx_equal_p (v->same->mult_val, v->mult_val)
                       && rtx_equal_p (v->same->add_val, v->add_val))

                {
                  v->dest_reg = v->same->dest_reg;
                  v->shared = 1;
                }

[ ... else if clauses skipped ... ]

              /* Store the value of dest_reg into the insn.  This sharing
                 will not be a problem as this insn will always be copied
                 later.  */

              *v->location = v->dest_reg;


And we do not initialize the second GIV's new register.  Leading
to the bogus code.


I believe all we need to do is clear v2->insn just like we clear
v2->same_insn when we determine the first GIV is invalid.

	* unroll.c (find_splittable_givs): If a GIV is an invalid
	address, clear any other GIVs which have been marked the
	same as the original GIV.

Index: unroll.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/./gcc/unroll.c,v
retrieving revision 1.23
diff -c -3 -p -r1.23 unroll.c
*** unroll.c	1998/05/06 21:07:32	1.23
--- unroll.c	1998/06/07 08:14:37
*************** find_splittable_givs (bl, unroll_type, l
*** 2949,2956 ****
  		      && ! verify_addresses (v, giv_inc, unroll_number))
  		    {
  		      for (v2 = v->next_iv; v2; v2 = v2->next_iv)
! 			if (v2->same_insn == v)
! 			  v2->same_insn = 0;
  
  		      if (loop_dump_stream)
  			fprintf (loop_dump_stream,
--- 2949,2960 ----
  		      && ! verify_addresses (v, giv_inc, unroll_number))
  		    {
  		      for (v2 = v->next_iv; v2; v2 = v2->next_iv)
! 			{
! 			  if (v2->same_insn == v)
! 			    v2->same_insn = 0;
! 			  if (v2->same == v)
! 			    v2->same = 0;
! 			}
  
  		      if (loop_dump_stream)
  			fprintf (loop_dump_stream,
*************** find_splittable_givs (bl, unroll_type, l
*** 2999,3006 ****
  		  if (! verify_addresses (v, giv_inc, unroll_number))
  		    {
  		      for (v2 = v->next_iv; v2; v2 = v2->next_iv)
! 			if (v2->same_insn == v)
! 			  v2->same_insn = 0;
  
  		      if (loop_dump_stream)
  			fprintf (loop_dump_stream,
--- 3003,3014 ----
  		  if (! verify_addresses (v, giv_inc, unroll_number))
  		    {
  		      for (v2 = v->next_iv; v2; v2 = v2->next_iv)
! 			{
! 			  if (v2->same_insn == v)
! 			    v2->same_insn = 0;
! 			  if (v2->same == v)
! 			    v2->same = 0;
! 			}
  
  		      if (loop_dump_stream)
  			fprintf (loop_dump_stream,





More information about the Gcc-patches mailing list