Problem with your 1999-11-23 change

Bernd Schmidt bernds@balti.cygnus.co.uk
Mon Dec 13 07:26:00 GMT 1999


On Mon, 13 Dec 1999, Mark Mitchell wrote:

> This change:
> 
>   1999-11-23  Bernd Schmidt  <bernds@cygnus.co.uk>
> 
> 	  * loop.c: Include "basic-block.h".
> 	  (try_copy_prop, replace_loop_reg): New functions.
> 	  (load_mems): Detect registers that just hold copies of the hoisted
> 	  mem, and call try_copy_prop to eliminate them.
> 	  * Makefile.in (loop.o): Update dependencies.
> 
> is causing the attached C++ test-case to crash with -O on
> x86-pc-linux-gnu.  In try_copy_prop, we're crashing here:
> 
> 	  if (REGNO_FIRST_UID (regno) == INSN_UID (insn))
> 	    store_is_first = 1;
> 
> because regno is 29, and the reg_n_info array only contains 29
> entries.  The REG in question is allocated eariler by load_mems.
> 
> I wasn't quite sure what you were intending here, so I'll let you
> straighten this out.

We're running into problems because we want to optimize a reg that was made
while processing an inner loop.  I've run into this kind of problem once too
often now, and I propose the patch below to fix it.

This patch bootstrapped on i586-linux and passed c-torture without additional
failures; I'll run the C++ tests later.  Note that I'm not entirely sure that
the range of insns we're scanning is large enough; I've looked at the code
in loop.c/unroll.c and found nothing that inserts insns after NEXT_INSN (end),
but it's possible that I've overlooked something.

If this patch goes in, we may be able to remove some cruft in the loop
optimizer that tests against max_reg_before_loop to avoid similar problems.
I'll look into this if the patch gets approved.

Where did the testcase come from?  Can we install it in the testsuite
(it could go into the C testsuite, it's plain C and failing with cc1).

Bernd

	* loop.c (loop_max_reg): New static variable.
	(loop_optimize): Initialize it.  Eliminate one unnecessary call to
	max_reg_num.
	(scan_loop): Call reg_scan_update whenever we may have added new
	registers, and update loop_max_reg.

Index: loop.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/loop.c,v
retrieving revision 1.207
diff -c -p -r1.207 loop.c
*** loop.c	1999/12/12 18:38:14	1.207
--- loop.c	1999/12/13 14:57:42
*************** static int num_mem_sets;
*** 213,218 ****
--- 213,221 ----
     A pseudo has valid regscan info if its number is < max_reg_before_loop.  */
  int max_reg_before_loop;
  
+ /* The value to pass to the next call of reg_scan_update.  */
+ static int loop_max_reg;
+ 
  /* This obstack is used in product_cheap_p to allocate its rtl.  It
     may call gen_reg_rtx which, in turn, may reallocate regno_reg_rtx.
     If we used the same obstack that it did, we would be deallocating
*************** loop_optimize (f, dumpfile, unroll_p, bc
*** 467,472 ****
--- 470,476 ----
    init_recog_no_volatile ();
  
    max_reg_before_loop = max_reg_num ();
+   loop_max_reg = max_reg_before_loop;
  
    regs_may_share = 0;
  
*************** loop_optimize (f, dumpfile, unroll_p, bc
*** 516,522 ****
    /* Now find all register lifetimes.  This must be done after
       find_and_verify_loops, because it might reorder the insns in the
       function.  */
!   reg_scan (f, max_reg_num (), 1);
  
    /* This must occur after reg_scan so that registers created by gcse
       will have entries in the register tables.
--- 520,526 ----
    /* Now find all register lifetimes.  This must be done after
       find_and_verify_loops, because it might reorder the insns in the
       function.  */
!   reg_scan (f, max_reg_before_loop, 1);
  
    /* This must occur after reg_scan so that registers created by gcse
       will have entries in the register tables.
*************** scan_loop (loop_start, end, loop_cont, u
*** 660,666 ****
    int insn_count;
    int in_libcall = 0;
    int tem;
!   rtx temp;
    /* The SET from an insn, if it is the only SET in the insn.  */
    rtx set, set1;
    /* Chain describing insns movable in current loop.  */
--- 664,670 ----
    int insn_count;
    int in_libcall = 0;
    int tem;
!   rtx temp, update_start, update_end;
    /* The SET from an insn, if it is the only SET in the insn.  */
    rtx set, set1;
    /* Chain describing insns movable in current loop.  */
*************** scan_loop (loop_start, end, loop_cont, u
*** 1178,1189 ****
--- 1182,1205 ----
    load_mems_and_recount_loop_regs_set (scan_start, end, loop_top,
  				       loop_start, &insn_count);
  
+   for (update_start = loop_start;
+        PREV_INSN (update_start) && GET_CODE (PREV_INSN (update_start)) != CODE_LABEL;
+        update_start = PREV_INSN (update_start))
+     ;
+   update_end = NEXT_INSN (end);
+ 
+   reg_scan_update (update_start, update_end, loop_max_reg);
+   loop_max_reg = max_reg_num ();
+ 
    if (flag_strength_reduce)
      {
        the_movables = movables;
        strength_reduce (scan_start, end, loop_top,
  		       insn_count, loop_start, end,
  		       loop_info, loop_cont, unroll_p, bct_p);
+ 
+       reg_scan_update (update_start, update_end, loop_max_reg);
+       loop_max_reg = max_reg_num ();
      }
  
    VARRAY_FREE (reg_single_usage);



More information about the Gcc-bugs mailing list