Re: 19991023 ICE compiling XFree86

[Richard Henderson <rth at cygnus dot com>]

On Wed, Oct 27, 1999 at 09:36:12AM +0930, Alan Modra wrote:
> resource.c: In function `GetResource':
> resource.c:386: Internal compiler error in `verify_local_live_at_start',
> at flow.c:2534

Looking at this closer, we find that GetResource is a function without a
declared return type, and that it doesn't return a value.  Changing the
function declaration to `void' makes the problem go away.

Since the function is implicitly defined to return `int', recent post-reload
flow changes determine that %eax is live at the end of the function, and
propogate that property back through the CFG.  As it happens, we decide that
%eax is live in block 12, the one that results in the problem above.

What happens is that resource.c begins at the first barrier before the target
insn (why it doesn't choose the first label, I don't know), and marches
forward through the "super-block" tracking register lifetimes.  In this case,
it begins in block 3 which, due to call patterns and CFG structure, does
not have the return value live, walks forward and determines a live-register
set that differs from flow's canonical version only in the state of the
return register.  

We then choose %eax as a good register to use, and die when we go to verify
lifetimes have not unexpectly changed.

So while I still don't agree with what resource.c is doing, I can't exactly
fault it.  Nor can I fault flow.c because, dammit, the register is live --
the fact that it contains garbage is the user's fault.

But we'd like to do something a bit more sane than an abort.  My current
preferred solution is to add clobbers to the instruction stream when we
determine that the user is doing something stupid like returning without
a value.  Then we don't get flow deciding that the return register is 
live throughout the entire program, so we don't get a discrepency in
lifetime information depending on the direction it's calculated in.

This change necessitated two minor related changes to reload and reg-stack
to accomodate the new insns.

Comments?


r~

	* function.c (diddle_return_value): New.
	(expand_function_end): Use it.
	* stmt.c (expand_null_return): Likewise.
	(expand_value_return): Likewise.

	* reg-stack.c (subst_stack_regs_pat): Handle clobbers at top-level.

	* reload1.c (reload): Don't remove return value clobbers.

Index: function.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/function.c,v
retrieving revision 1.128
diff -c -p -d -r1.128 function.c
*** function.c	1999/11/01 01:11:20	1.128
--- function.c	1999/11/03 22:17:08
*************** expand_dummy_function_end ()
*** 6150,6155 ****
--- 6150,6185 ----
    current_function = 0;
  }
  
+ /* Emit CODE for each register of the return value.  Useful values for
+    code are USE and CLOBBER.  */
+ 
+ void
+ diddle_return_value (code)
+      enum rtx_code code;
+ {
+   rtx return_reg = DECL_RTL (DECL_RESULT (current_function_decl));
+ 
+   if (return_reg)
+     {
+       if (GET_CODE (return_reg) == REG
+ 	  && REGNO (return_reg) < FIRST_PSEUDO_REGISTER)
+ 	emit_insn (gen_rtx_fmt_e (code, VOIDmode, return_reg));
+       else if (GET_CODE (return_reg) == PARALLEL)
+ 	{
+ 	  int i;
+ 
+ 	  for (i = 0; i < XVECLEN (return_reg, 0); i++)
+ 	    {
+ 	      rtx x = XEXP (XVECEXP (return_reg, 0, i), 0);
+ 
+ 	      if (GET_CODE (x) == REG
+ 		  && REGNO (x) < FIRST_PSEUDO_REGISTER)
+ 		emit_insn (gen_rtx_fmt_e (code, VOIDmode, x));
+ 	    }
+ 	}
+     }
+ }
+ 
  /* Generate RTL for the end of the current function.
     FILENAME and LINE are the current position in the source file. 
  
*************** expand_function_end (filename, line, end
*** 6332,6338 ****
       structure returning.  */
  
    if (return_label)
!     emit_label (return_label);
  
    /* C++ uses this.  */
    if (end_bindings)
--- 6362,6377 ----
       structure returning.  */
  
    if (return_label)
!     {
!       /* Before the return label, clobber the return registers so that
!          they are not propogated live to the rest of the function.  This
! 	 can only happen with functions that drop through; if there had
! 	 been a return statement, there would have either been a return
! 	 rtx, or a jump to the return label.  */
!       diddle_return_value (CLOBBER);
! 
!       emit_label (return_label);
!     }
  
    /* C++ uses this.  */
    if (end_bindings)
Index: function.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/function.h,v
retrieving revision 1.34
diff -c -p -d -r1.34 function.h
*** function.h	1999/09/26 18:13:28	1.34
--- function.h	1999/11/03 22:17:08
*************** extern void free_expr_status		PROTO((str
*** 578,583 ****
--- 578,585 ----
  
  extern rtx get_first_block_beg		PROTO((void));
  
+ extern void diddle_return_value		PROTO((enum rtx_code));
+ 
  extern void init_virtual_regs		PROTO((struct emit_status *));
  
  /* Called once, at initialization, to initialize function.c.  */
Index: reg-stack.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/reg-stack.c,v
retrieving revision 1.45
diff -c -p -d -r1.45 reg-stack.c
*** reg-stack.c	1999/11/02 02:09:55	1.45
--- reg-stack.c	1999/11/03 22:17:08
*************** subst_stack_regs_pat (insn, regstack, pa
*** 1394,1420 ****
        {
  	rtx note;
  
- 	/* The fix_truncdi_1 pattern wants to be able to allocate it's
- 	   own scratch register.  It does this by clobbering an fp reg
- 	   so that it is assured of an empty reg-stack register.
- 	   If the register is live, kill it now.  Remove the DEAD/UNUSED
- 	   note so we don't try to kill it later too.  */
- 
  	dest = get_true_reg (&XEXP (pat, 0));
  	if (STACK_REG_P (*dest))
  	  {
  	    note = find_reg_note (insn, REG_DEAD, *dest);
! 	    if (note)
! 	      emit_pop_insn (insn, regstack, *dest, EMIT_BEFORE);
! 	    else
  	      {
! 		note = find_reg_note (insn, REG_UNUSED, *dest);
! 		if (!note)
! 		  abort ();
  	      }
  
! 	    remove_note (insn, note);
! 	    replace_reg (dest, LAST_STACK_REG);
  	  }
  	break;
        }
--- 1394,1441 ----
        {
  	rtx note;
  
  	dest = get_true_reg (&XEXP (pat, 0));
  	if (STACK_REG_P (*dest))
  	  {
  	    note = find_reg_note (insn, REG_DEAD, *dest);
! 
! 	    if (pat != PATTERN (insn))
  	      {
! 		/* The fix_truncdi_1 pattern wants to be able to allocate
! 		   it's own scratch register.  It does this by clobbering
! 		   an fp reg so that it is assured of an empty reg-stack
! 		   register.  If the register is live, kill it now. 
! 		   Remove the DEAD/UNUSED note so we don't try to kill it
! 		   later too.  */
! 
! 		if (note)
! 		  emit_pop_insn (insn, regstack, *dest, EMIT_BEFORE);
! 		else
! 		  {
! 		    note = find_reg_note (insn, REG_UNUSED, *dest);
! 		    if (!note)
! 		      abort ();
! 		  }
! 		remove_note (insn, note);
! 		replace_reg (dest, LAST_STACK_REG);
  	      }
+ 	    else
+ 	      {
+ 		/* A top-level clobber with no REG_DEAD, and no hard-regnum
+ 		   indicates an uninitialized value.  Because reload removed
+ 		   all other clobbers, this must be due to a function 
+ 		   returning without a value.  Load up a NaN.  */
  
! 		if (! note
! 		    && get_hard_regnum (regstack, *dest) == -1)
! 		  {
! 		    pat = gen_rtx_SET (VOIDmode,
! 				       FP_MODE_REG (REGNO (*dest), SFmode),
! 				       nan);
! 		    PATTERN (insn) = pat;
! 		    move_for_stack_reg (insn, regstack, pat);
! 		  }
! 	      }
  	  }
  	break;
        }
Index: reload1.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/reload1.c,v
retrieving revision 1.179
diff -c -p -d -r1.179 reload1.c
*** reload1.c	1999/11/03 11:23:27	1.179
--- reload1.c	1999/11/03 22:17:09
*************** reload (first, global, dumpfile)
*** 1106,1118 ****
       which are only valid during and after reload.  */
    reload_completed = 1;
  
!   /* Make a pass over all the insns and delete all USEs which we
!      inserted only to tag a REG_EQUAL note on them.  Remove all
!      REG_DEAD and REG_UNUSED notes.  Delete all CLOBBER insns and
!      simplify (subreg (reg)) operands.  Also remove all REG_RETVAL and
!      REG_LIBCALL notes since they are no longer useful or accurate.
!      Strip and regenerate REG_INC notes that may have been moved
!      around.  */
  
    for (insn = first; insn; insn = NEXT_INSN (insn))
      if (GET_RTX_CLASS (GET_CODE (insn)) == 'i')
--- 1106,1117 ----
       which are only valid during and after reload.  */
    reload_completed = 1;
  
!   /* Make a pass over all the insns and delete all USEs which we inserted
!      only to tag a REG_EQUAL note on them.  Remove all REG_DEAD and REG_UNUSED
!      notes.  Delete all CLOBBER insns that don't refer to the return value
!      and simplify (subreg (reg)) operands.  Also remove all REG_RETVAL and
!      REG_LIBCALL notes since they are no longer useful or accurate.  Strip
!      and regenerate REG_INC notes that may have been moved around.  */
  
    for (insn = first; insn; insn = NEXT_INSN (insn))
      if (GET_RTX_CLASS (GET_CODE (insn)) == 'i')
*************** reload (first, global, dumpfile)
*** 1121,1127 ****
  
  	if ((GET_CODE (PATTERN (insn)) == USE
  	     && find_reg_note (insn, REG_EQUAL, NULL_RTX))
! 	    || GET_CODE (PATTERN (insn)) == CLOBBER)
  	  {
  	    PUT_CODE (insn, NOTE);
  	    NOTE_SOURCE_FILE (insn) = 0;
--- 1120,1128 ----
  
  	if ((GET_CODE (PATTERN (insn)) == USE
  	     && find_reg_note (insn, REG_EQUAL, NULL_RTX))
! 	    || (GET_CODE (PATTERN (insn)) == CLOBBER
! 		&& (GET_CODE (XEXP (PATTERN (insn), 0)) != REG
! 		    || ! REG_FUNCTION_VALUE_P (XEXP (PATTERN (insn), 0)))))
  	  {
  	    PUT_CODE (insn, NOTE);
  	    NOTE_SOURCE_FILE (insn) = 0;
Index: stmt.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/stmt.c,v
retrieving revision 1.106
diff -c -p -d -r1.106 stmt.c
*** stmt.c	1999/11/01 04:24:33	1.106
--- stmt.c	1999/11/03 22:17:09
*************** void
*** 2665,2671 ****
  expand_null_return ()
  {
    struct nesting *block = block_stack;
!   rtx last_insn = 0;
  
    /* Does any pending block have cleanups?  */
  
--- 2665,2677 ----
  expand_null_return ()
  {
    struct nesting *block = block_stack;
!   rtx last_insn = get_last_insn ();
! 
!   /* If this function was declared to return a value, but we 
!      didn't, clobber the return registers so that they are not
!      propogated live to the rest of the function.  */
! 
!   diddle_return_value (CLOBBER);
  
    /* Does any pending block have cleanups?  */
  
*************** expand_value_return (val)
*** 2709,2734 ****
        else
  	emit_move_insn (return_reg, val);
      }
- 
-   if (GET_CODE (return_reg) == REG
-       && REGNO (return_reg) < FIRST_PSEUDO_REGISTER)
-     emit_insn (gen_rtx_USE (VOIDmode, return_reg));
- 
-   /* Handle calls that return values in multiple non-contiguous locations.
-      The Irix 6 ABI has examples of this.  */
-   else if (GET_CODE (return_reg) == PARALLEL)
-     {
-       int i;
- 
-       for (i = 0; i < XVECLEN (return_reg, 0); i++)
- 	{
- 	  rtx x = XEXP (XVECEXP (return_reg, 0, i), 0);
  
! 	  if (GET_CODE (x) == REG
! 	      && REGNO (x) < FIRST_PSEUDO_REGISTER)
! 	    emit_insn (gen_rtx_USE (VOIDmode, x));
! 	}
!     }
  
    /* Does any pending block have cleanups?  */
  
--- 2715,2722 ----
        else
  	emit_move_insn (return_reg, val);
      }
  
!   diddle_return_value (USE);
  
    /* Does any pending block have cleanups?  */