public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: Corey Minyard <minyard@acm.org>
To: Richard Henderson <rth@redhat.com>
Cc: gcc@gcc.gnu.org
Subject: Re: Loop optimization bug with Ada front end on PPC (and probably Alpha)
Date: Fri, 16 Nov 2001 03:42:00 -0000	[thread overview]
Message-ID: <3C01C4F3.2020403@acm.org> (raw)
In-Reply-To: <20011125175500.A11474@redhat.com>

That patch looks a lot better than mine :-).  However, I'm getting a 
segv compiling unwind-dw2-fde.c.  I put a print in (I can't use gdb 5.0 
with the output of gcc 3.0 on Linux PowerPC), and it's dying in the line: 

  && INSN_IN_RANGE_P (JUMP_LABEL (insn), loop->top, loop->cont)

It looks like loop->top is alway NULL.  I'm working with a slightly old 
CVS, maybe I should update, or is this a real problem?

-Corey

Richard Henderson wrote:

>The problem is not limited to doloop -- the unroller ought
>to hit this problem as well.  I'm testing the following.
>
>
>
>r~
>
>
>
>Index: loop.c
>===================================================================
>RCS file: /cvs/gcc/gcc/gcc/loop.c,v
>retrieving revision 1.369
>diff -c -p -d -r1.369 loop.c
>*** loop.c	2001/11/15 23:44:56	1.369
>--- loop.c	2001/11/26 01:52:34
>*************** typedef struct loop_replace_args
>*** 282,293 ****
>    rtx insn;
>  } loop_replace_args;
>  
>- /* Nonzero iff INSN is between START and END, inclusive.  */
>- #define INSN_IN_RANGE_P(INSN, START, END)	\
>-   (INSN_UID (INSN) < max_uid_for_loop		\
>-    && INSN_LUID (INSN) >= INSN_LUID (START)	\
>-    && INSN_LUID (INSN) <= INSN_LUID (END))
>- 
>  /* Indirect_jump_in_function is computed once per function.  */
>  static int indirect_jump_in_function;
>  static int indirect_jump_in_function_p PARAMS ((rtx));
>--- 282,287 ----
>Index: loop.h
>===================================================================
>RCS file: /cvs/gcc/gcc/gcc/loop.h,v
>retrieving revision 1.56
>diff -c -p -d -r1.56 loop.h
>*** loop.h	2001/10/29 22:13:40	1.56
>--- loop.h	2001/11/26 01:52:34
>*************** Software Foundation, 59 Temple Place - S
>*** 50,55 ****
>--- 50,60 ----
>  #define REGNO_FIRST_LUID(REGNO) uid_luid[REGNO_FIRST_UID (REGNO)]
>  #define REGNO_LAST_LUID(REGNO) uid_luid[REGNO_LAST_UID (REGNO)]
>  
>+ /* Nonzero iff INSN is between START and END, inclusive.  */
>+ #define INSN_IN_RANGE_P(INSN, START, END)	\
>+   (INSN_UID (INSN) < max_uid_for_loop		\
>+    && INSN_LUID (INSN) >= INSN_LUID (START)	\
>+    && INSN_LUID (INSN) <= INSN_LUID (END))
>  
>  /* A "basic induction variable" or biv is a pseudo reg that is set
>     (within this loop) only by incrementing or decrementing it.  */
>Index: unroll.c
>===================================================================
>RCS file: /cvs/gcc/gcc/gcc/unroll.c,v
>retrieving revision 1.147
>diff -c -p -d -r1.147 unroll.c
>*** unroll.c	2001/11/21 00:50:56	1.147
>--- unroll.c	2001/11/26 01:52:34
>*************** loop_iterations (loop)
>*** 3480,3485 ****
>--- 3480,3486 ----
>    int unsigned_p, compare_dir, final_larger;
>    rtx last_loop_insn;
>    rtx reg_term;
>+   rtx insn;
>    struct iv_class *bl;
>  
>    loop_info->n_iterations = 0;
>*************** loop_iterations (loop)
>*** 3705,3710 ****
>--- 3706,3762 ----
>  
>    if (initial_value == 0)
>      return 0;
>+ 
>+   /* Some code transformations can result in code akin to
>+ 
>+ 	LOOP_BEG
>+ 	  goto start;
>+ 	top:
>+ 	  i++;
>+ 	start:
>+ 	  ...
>+ 	LOOP_CONT
>+ 	  if (i < n) goto top;
>+ 	LOOP_END
>+ 
>+      In this situation, we skip the increment the first time through
>+      the loop, which results in an incorrect estimate of the number
>+      of iterations.  As we did for GIVs above, adjust the initial value
>+      to compensate.  */
>+ 
>+   off_by_one = 0;
>+   for (insn = loop->start; insn != loop->top; insn = NEXT_INSN (insn))
>+     if (GET_CODE (insn) == JUMP_INSN)
>+       {
>+ 	if (any_uncondjump_p (insn)
>+ 	    && JUMP_LABEL (insn)
>+ 	    && INSN_IN_RANGE_P (JUMP_LABEL (insn), loop->top, loop->cont))
>+ 	  {
>+ 	    if (reg_set_between_p (bl->biv->src_reg, insn, JUMP_LABEL (insn)))
>+ 	      off_by_one = 1;
>+ 	    break;
>+ 	  }
>+ 	/* No idea what's going on.  */
>+ 	if (loop_dump_stream)
>+ 	  fprintf (loop_dump_stream,
>+ 		   "Loop iterations: Confused by jump before loop top.\n");
>+ 	return 0;
>+       }
>+ 
>+   if (off_by_one)
>+     {
>+       if (loop_dump_stream)
>+ 	fprintf (loop_dump_stream,
>+ 		 "Loop iterations: Basic induction var skips initial incr.\n");
>+       if (GET_CODE (increment) != CONST_INT)
>+ 	{
>+ 	  if (loop_dump_stream)
>+ 	    fprintf (loop_dump_stream,
>+ 		     "Loop iterations: Can't adjust with non-constant incr.\n");
>+ 	  return 0;
>+ 	}
>+       initial_value = plus_constant (initial_value, -INTVAL (increment));
>+     }
>  
>    unsigned_p = 0;
>    off_by_one = 0;
>



WARNING: multiple messages have this Message-ID
From: Corey Minyard <minyard@acm.org>
To: Richard Henderson <rth@redhat.com>
Cc: gcc@gcc.gnu.org
Subject: Re: Loop optimization bug with Ada front end on PPC (and probably Alpha)
Date: Sun, 25 Nov 2001 20:27:00 -0000	[thread overview]
Message-ID: <3C01C4F3.2020403@acm.org> (raw)
Message-ID: <20011125202700.jamJQYhOgOxswuevWTkbnP9TVaCDi13BP173H702Jbw@z> (raw)
In-Reply-To: <20011125175500.A11474@redhat.com>

That patch looks a lot better than mine :-).  However, I'm getting a 
segv compiling unwind-dw2-fde.c.  I put a print in (I can't use gdb 5.0 
with the output of gcc 3.0 on Linux PowerPC), and it's dying in the line: 

  && INSN_IN_RANGE_P (JUMP_LABEL (insn), loop->top, loop->cont)

It looks like loop->top is alway NULL.  I'm working with a slightly old 
CVS, maybe I should update, or is this a real problem?

-Corey

Richard Henderson wrote:

>The problem is not limited to doloop -- the unroller ought
>to hit this problem as well.  I'm testing the following.
>
>
>
>r~
>
>
>
>Index: loop.c
>===================================================================
>RCS file: /cvs/gcc/gcc/gcc/loop.c,v
>retrieving revision 1.369
>diff -c -p -d -r1.369 loop.c
>*** loop.c	2001/11/15 23:44:56	1.369
>--- loop.c	2001/11/26 01:52:34
>*************** typedef struct loop_replace_args
>*** 282,293 ****
>    rtx insn;
>  } loop_replace_args;
>  
>- /* Nonzero iff INSN is between START and END, inclusive.  */
>- #define INSN_IN_RANGE_P(INSN, START, END)	\
>-   (INSN_UID (INSN) < max_uid_for_loop		\
>-    && INSN_LUID (INSN) >= INSN_LUID (START)	\
>-    && INSN_LUID (INSN) <= INSN_LUID (END))
>- 
>  /* Indirect_jump_in_function is computed once per function.  */
>  static int indirect_jump_in_function;
>  static int indirect_jump_in_function_p PARAMS ((rtx));
>--- 282,287 ----
>Index: loop.h
>===================================================================
>RCS file: /cvs/gcc/gcc/gcc/loop.h,v
>retrieving revision 1.56
>diff -c -p -d -r1.56 loop.h
>*** loop.h	2001/10/29 22:13:40	1.56
>--- loop.h	2001/11/26 01:52:34
>*************** Software Foundation, 59 Temple Place - S
>*** 50,55 ****
>--- 50,60 ----
>  #define REGNO_FIRST_LUID(REGNO) uid_luid[REGNO_FIRST_UID (REGNO)]
>  #define REGNO_LAST_LUID(REGNO) uid_luid[REGNO_LAST_UID (REGNO)]
>  
>+ /* Nonzero iff INSN is between START and END, inclusive.  */
>+ #define INSN_IN_RANGE_P(INSN, START, END)	\
>+   (INSN_UID (INSN) < max_uid_for_loop		\
>+    && INSN_LUID (INSN) >= INSN_LUID (START)	\
>+    && INSN_LUID (INSN) <= INSN_LUID (END))
>  
>  /* A "basic induction variable" or biv is a pseudo reg that is set
>     (within this loop) only by incrementing or decrementing it.  */
>Index: unroll.c
>===================================================================
>RCS file: /cvs/gcc/gcc/gcc/unroll.c,v
>retrieving revision 1.147
>diff -c -p -d -r1.147 unroll.c
>*** unroll.c	2001/11/21 00:50:56	1.147
>--- unroll.c	2001/11/26 01:52:34
>*************** loop_iterations (loop)
>*** 3480,3485 ****
>--- 3480,3486 ----
>    int unsigned_p, compare_dir, final_larger;
>    rtx last_loop_insn;
>    rtx reg_term;
>+   rtx insn;
>    struct iv_class *bl;
>  
>    loop_info->n_iterations = 0;
>*************** loop_iterations (loop)
>*** 3705,3710 ****
>--- 3706,3762 ----
>  
>    if (initial_value == 0)
>      return 0;
>+ 
>+   /* Some code transformations can result in code akin to
>+ 
>+ 	LOOP_BEG
>+ 	  goto start;
>+ 	top:
>+ 	  i++;
>+ 	start:
>+ 	  ...
>+ 	LOOP_CONT
>+ 	  if (i < n) goto top;
>+ 	LOOP_END
>+ 
>+      In this situation, we skip the increment the first time through
>+      the loop, which results in an incorrect estimate of the number
>+      of iterations.  As we did for GIVs above, adjust the initial value
>+      to compensate.  */
>+ 
>+   off_by_one = 0;
>+   for (insn = loop->start; insn != loop->top; insn = NEXT_INSN (insn))
>+     if (GET_CODE (insn) == JUMP_INSN)
>+       {
>+ 	if (any_uncondjump_p (insn)
>+ 	    && JUMP_LABEL (insn)
>+ 	    && INSN_IN_RANGE_P (JUMP_LABEL (insn), loop->top, loop->cont))
>+ 	  {
>+ 	    if (reg_set_between_p (bl->biv->src_reg, insn, JUMP_LABEL (insn)))
>+ 	      off_by_one = 1;
>+ 	    break;
>+ 	  }
>+ 	/* No idea what's going on.  */
>+ 	if (loop_dump_stream)
>+ 	  fprintf (loop_dump_stream,
>+ 		   "Loop iterations: Confused by jump before loop top.\n");
>+ 	return 0;
>+       }
>+ 
>+   if (off_by_one)
>+     {
>+       if (loop_dump_stream)
>+ 	fprintf (loop_dump_stream,
>+ 		 "Loop iterations: Basic induction var skips initial incr.\n");
>+       if (GET_CODE (increment) != CONST_INT)
>+ 	{
>+ 	  if (loop_dump_stream)
>+ 	    fprintf (loop_dump_stream,
>+ 		     "Loop iterations: Can't adjust with non-constant incr.\n");
>+ 	  return 0;
>+ 	}
>+       initial_value = plus_constant (initial_value, -INTVAL (increment));
>+     }
>  
>    unsigned_p = 0;
>    off_by_one = 0;
>



  reply	other threads:[~2001-11-26  4:27 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-11-13  5:20 Corey Minyard
2001-11-13  6:05 ` David Edelsohn
2001-11-14  8:05   ` Corey Minyard
2001-11-13  8:02 ` guerby
2001-11-13  9:48   ` Corey Minyard
2001-11-16 22:19     ` David Edelsohn
2001-11-26  7:39       ` David Edelsohn
2001-11-15  0:48 ` Richard Henderson
2001-11-15 14:19   ` Andreas Schwab
2001-11-15 16:20     ` Richard Henderson
2001-11-25 12:23       ` Richard Henderson
2001-11-18  9:18     ` Richard Henderson
2001-11-27  0:02       ` Richard Henderson
2001-11-25  8:57     ` Andreas Schwab
2001-11-15 18:02   ` Corey Minyard
2001-11-15 19:37     ` Richard Henderson
2001-11-25 15:52       ` Richard Henderson
2001-11-16  3:13     ` Richard Henderson
2001-11-16  3:42       ` Corey Minyard [this message]
2001-11-16  8:54         ` Bryce McKinlay
2001-11-26  0:47           ` Bryce McKinlay
2001-11-25 20:27         ` Corey Minyard
2001-11-17  1:33       ` Corey Minyard
2001-11-17  6:09         ` Corey Minyard
2001-11-26 10:22           ` Corey Minyard
2001-11-17 11:51         ` Richard Henderson
2001-11-17 15:20           ` Corey Minyard
2001-11-26 14:45             ` Corey Minyard
2001-11-18  5:15           ` Corey Minyard
2001-11-18  8:59             ` Richard Henderson
2001-11-19  2:58               ` Corey Minyard
2001-11-19  3:11                 ` Richard Henderson
2001-11-19  5:36                   ` Corey Minyard
2001-11-19  7:48                     ` Richard Henderson
2001-11-19  7:58                       ` Corey Minyard
2001-11-19  9:43                         ` Richard Henderson
2001-11-19 12:44                           ` Corey Minyard
2001-11-19 13:53                             ` Richard Henderson
2001-11-20 11:20                               ` Corey Minyard
2001-11-27 19:57                                 ` Corey Minyard
2001-11-27 15:31                               ` Richard Henderson
2001-11-27 15:04                             ` Corey Minyard
2001-11-27 14:27                           ` Richard Henderson
2001-11-27 13:25                         ` Corey Minyard
2001-11-27 12:58                       ` Richard Henderson
2001-11-27 11:51                     ` Corey Minyard
2001-11-27 10:29                   ` Richard Henderson
2001-11-27 10:08                 ` Corey Minyard
2001-11-26 23:18               ` Richard Henderson
2001-11-26 17:15             ` Corey Minyard
2001-11-26 13:49           ` Richard Henderson
2001-11-26  8:58         ` Corey Minyard
2001-11-25 17:58       ` Richard Henderson
2001-11-25 15:06     ` Corey Minyard
2001-11-15 18:47 Richard Kenner
2001-11-25 15:13 ` Richard Kenner
2001-11-15 19:05 dewar
2001-11-25 15:20 ` dewar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3C01C4F3.2020403@acm.org \
    --to=minyard@acm.org \
    --cc=gcc@gcc.gnu.org \
    --cc=rth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).