[RFA:] Fix g77.f-torture/compile/970915-0.f, -Os verify_local_live_at_start MMIX

Hans-Peter Nilsson hp@bitrange.com
Tue Apr 16 00:14:00 GMT 2002


At the moment, even the following reduced test-case trigs the
error at -O2.  The bug is highly dependant on the number of
generated pseudos and mem references and optimizations to
change them done before cfg.

      SUBROUTINE TSTSIG11
      IMPLICIT COMPLEX (A-Z)
      branch3 =  sw2/cw+sw2*tw**2+(1.+1./tauw)*tw**2-1./tauw
      RETURN
      END

This code apparently (by inspection of RTL ;-) computes
something unused from several uninitialized SCmode variables
(expanding to SFmode pairs).  The whole thing could be optimized
out.  The reason that does not happen is partially current
limitations on removing the (fortran) libcalls involved.
Strange is that the fortran front-end decides to do complex
computations differently depending on -Os.  Tweaking to
consistently emit that same RTL as for -Os makes the bug
apparent with other optimization levels.

One reason this only happens for MMIX for
g77.f-torture/compile/970915-0.f is that a stack slot is
generated for each and every SF-DF conversion.  All float
computations are performed in DFmode, every time extending from
SF:s and truncating by the memory-only LDSF insn and STSF insns.
(These are the only SFmode insns.)  The multitude of stack
slots, apparently just above 100 in this case, are a wart.
However, I don't want to tweak mmix.md because [1] floats are
unimportant and [2] IMHO it falls on gcc machinery to make it
easy to generate a short-lived stack-slot (that can be re-used).

On to the bug.  As I recently mentioned in
<URL:http://gcc.gnu.org/ml/gcc/2002-04/msg00669.html>, life info
will be inconsistent if the limit is reached on mems checked for
lifeness in add_to_mem_set_list, and subsequent mems are dropped
on the floor.  This limit was introduced in
<URL:http://gcc.gnu.org/ml/gcc-patches/2001-01/msg01329.html>
apparently for good reasons.  Subsequent optimizations local to
a basic block to remove one mem may make another mem fit in the
list, so the pseudo the latter is set from is suddenly
discovered to actually be dead.  (Nightmarish horror!)  The rest
of the life-info machinery is not aware of that.  I use the
BB_DIRTY to signal this inconsistency, since that flag is there
for purpose.  Though it might be surprising to have life info
"dirty" right when it's initially computed, I think this is the
most reasonable fix.

The verify_local_live_at_start change is actually just to build
awareness of the possible inconsistency.  I don't think it can
currently make a difference for codes like the test-case even if
some BB was flagged BB_DIRTY after update_life_info, because
there are several calls to clear_bb_flags before the first
update_life_info_in_dirty_blocks call.  Speaking of
update_life_info_in_dirty_blocks, I can't see where BB_DIRTY
would be reset before this patch.

I'm sad to say that this would not fix PR 6123.  The
MAX_MEM_SET_LIST_LEN limit isn't reached for that case.
(Building cc1 for sparc and testing that I get the same ICE of
course.)

I'll update this patch for 3.1 if this is on the right route;
3.1 doesn't have BB_DIRTY and clear_bb_flags but I think that's
all.

Bootstrap and checked with no regressions on i686-pc-linux-gnu.
Cross-tested with no regressions on mmix-knuth-mmixware,
cris-axis-elf, v850-unknown-elf, i960-unknown-coff,
m32r-unknown-elf, mn10300-unknown-elf and arm-unknown-elf.

Ok to commit?

	* flow.c (verify_local_live_at_start): Don't abort if registers
	die but life info isn't complete.
	(update_life_info): Continue looping if life info is incomplete
	and propagate_block indicates that something changed.  Call
	clear_bb_flags first thing in loop.
	(add_to_mem_set_list): If pbi->mem_set_list is full, signal
	incomplete life info for this basic block by BB_DIRTY.

Index: flow.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/flow.c,v
retrieving revision 1.514
diff -p -c -r1.514 flow.c
*** flow.c	12 Apr 2002 07:52:02 -0000	1.514
--- flow.c	16 Apr 2002 02:58:31 -0000
*************** verify_local_live_at_start (new_live_at_
*** 593,600 ****

        EXECUTE_IF_SET_IN_REG_SET (new_live_at_start, 0, i,
  	{
!           /* No registers should die.  */
! 	  if (REGNO_REG_SET_P (bb->global_live_at_start, i))
  	    {
  	      if (rtl_dump_file)
  		{
--- 593,601 ----

        EXECUTE_IF_SET_IN_REG_SET (new_live_at_start, 0, i,
  	{
!           /* No registers should die if life info is complete.  */
! 	  if (REGNO_REG_SET_P (bb->global_live_at_start, i)
! 	      && (bb->flags & BB_DIRTY) != 0)
  	    {
  	      if (rtl_dump_file)
  		{
*************** update_life_info (blocks, extent, prop_f
*** 656,662 ****
--- 657,665 ----
        for ( ; ; )
  	{
  	  int changed = 0;
+ 	  int incomplete_life_info = 0;

+ 	  clear_bb_flags ();
  	  calculate_global_regs_live (blocks, blocks,
  				prop_flags & (PROP_SCAN_DEAD_CODE
  					      | PROP_ALLOW_CFG_CHANGES));
*************** update_life_info (blocks, extent, prop_f
*** 675,683 ****
  	      changed |= propagate_block (bb, tmp, NULL, NULL,
  				prop_flags & (PROP_SCAN_DEAD_CODE
  					      | PROP_KILL_DEAD_CODE));
  	    }

! 	  if (! changed || ! cleanup_cfg (CLEANUP_EXPENSIVE))
  	    break;
  	}

--- 678,693 ----
  	      changed |= propagate_block (bb, tmp, NULL, NULL,
  				prop_flags & (PROP_SCAN_DEAD_CODE
  					      | PROP_KILL_DEAD_CODE));
+ 	      incomplete_life_info |= (bb->flags & BB_DIRTY) != 0;
  	    }

! 	  /* If proper life info couldn't previously be built, and
! 	     something changed, then it's possible that we can build it
! 	     properly on the next round and then have further progress.
! 	     That will not depend on whether there was cfg cleanup.  */
! 	  if (!changed
! 	      || (!cleanup_cfg (CLEANUP_EXPENSIVE)
! 		  && !incomplete_life_info))
  	    break;
  	}

*************** add_to_mem_set_list (pbi, mem)
*** 2394,2399 ****
--- 2404,2413 ----
        pbi->mem_set_list = alloc_EXPR_LIST (0, mem, pbi->mem_set_list);
        pbi->mem_set_list_len++;
      }
+   else
+     /* We can't provide proper life info if we can't add more items to the
+        list.  Mark this bb as having incomplete life info.  */
+     pbi->bb->flags |= BB_DIRTY;
  }

  /* INSN references memory, possibly using autoincrement addressing modes.


brgds, H-P



More information about the Gcc-patches mailing list