[patch] PR19616, take 2

[Steven Bosscher <stevenb at suse dot de>]

Hi,

The following patch reverts the code pessimizing patch for PR15387
and replaces it with a different approach.

The problem was that a stack slot for a sibcall argument was being
overwritten by another argument on ix86.  Consider the following
tree dumnp:

sister (f, b) 
{ 
  int * tmp; 
  int T.2; 
  int T.1; 
  int * f.0; 
 
<bb 0>: 
  f.0 = (int *)&f; 
  brother (*(f.0 + 4B) == b, b) [tail call]; 
  return; 
 
} 

That resulted in this RTL: 
 
(insn 3 8 4 0 (set (reg/v:SI 59 [ b ]) 
        (mem/i:SI (plus:SI (reg/f:SI 53 virtual-incoming-args) 
                (const_int 12 [0xc])) [2 b+0 S4 A32])) -1 (nil) 
    (expr_list:REG_EQUIV (mem/i:SI (plus:SI (reg/f:SI 53 virtual-incoming-args) 
                (const_int 12 [0xc])) [2 b+0 S4 A32]) 
        (nil))) 
 
(insn 11 9 13 1 (set (reg:SI 58 [ f.0 ]) 
        (reg/f:SI 53 virtual-incoming-args)) -1 (nil) 
    (nil)) 
 
(insn 15 14 16 1 (set (mem/i:SI (plus:SI (reg/f:SI 53 virtual-incoming-args) 
                (const_int 4 [0x4])) [0 S4 A32]) 
        (reg/v:SI 59 [ b ])) -1 (nil) 
    (nil)) 
 
(insn 16 15 17 1 (set (reg:SI 61) 
        (mem:SI (plus:SI (reg:SI 58 [ f.0 ]) 
                (const_int 4 [0x4])) [2 S4 A32])) -1 (nil) 
    (nil)) 
 
So reg 61 gets set to the 'b', which was just stored in *(f.0 + 4) to
pass it to "brother".  The code in GCC checks that the evaluation of
an argument to pass does not clobber another argument slot, but it just
looks if something may be stored to a MEM with virtual-incoming-args as
the base.  In this case, f0 == virtual-incoming-arg but the calls.c
code can't see that.

The fix Zdenek applied was to assume all sets to a MEM could cause this
problem, but actually it is only a problem in cases where an argument
of the caller has its address taken and that it would be used somehow
in an expression for an argument for the callee, ie. if evaluating the
argument of the callee has side-effects.

A less pessimistic fix is to just disallow TER to propagate expressions
into the argument list of a call that may be a tail call.  That way, to
expand all the arguments will be GIMPLE registers, and the arguments
are always evaluated before setting up the arguments for the sibcall.

Successfully bootstrapped and tested on x86_64-suse-linux-gnu.  I have
tests running on other platforms as well, just to be sure, but I think
this is the right fix.
To be sure I have also checked that the test case from PR15387 failed
with GCC "3.5.0 20040728 (experimental)" and passes if apply this patch.

OK for mainline?

Gr.
Steven

	* tree.h (CALL_EXPR_TAILCALL): Add comment.

	* calls.c (check_sibcall_argument_overlap_1): Revert the change
	to this function from 2004-07-10.
	* tree-outof-ssa.c (find_replaceable_in_bb): Do not replace into
	the arguments of tail call candidates.

Index: calls.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/calls.c,v
retrieving revision 1.377
diff -c -3 -p -r1.377 calls.c
*** calls.c	18 Jan 2005 23:06:55 -0000	1.377
--- calls.c	25 Jan 2005 18:59:50 -0000
*************** check_sibcall_argument_overlap_1 (rtx x)
*** 1670,1676 ****
  	       && GET_CODE (XEXP (XEXP (x, 0), 1)) == CONST_INT)
  	i = INTVAL (XEXP (XEXP (x, 0), 1));
        else
! 	return 1;
  
  #ifdef ARGS_GROW_DOWNWARD
        i = -i - GET_MODE_SIZE (GET_MODE (x));
--- 1670,1676 ----
  	       && GET_CODE (XEXP (XEXP (x, 0), 1)) == CONST_INT)
  	i = INTVAL (XEXP (XEXP (x, 0), 1));
        else
! 	return 0;
  
  #ifdef ARGS_GROW_DOWNWARD
        i = -i - GET_MODE_SIZE (GET_MODE (x));
Index: tree-outof-ssa.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-outof-ssa.c,v
retrieving revision 2.41
diff -c -3 -p -r2.41 tree-outof-ssa.c
*** tree-outof-ssa.c	23 Jan 2005 15:05:31 -0000	2.41
--- tree-outof-ssa.c	25 Jan 2005 18:59:51 -0000
*************** static void
*** 1626,1632 ****
  find_replaceable_in_bb (temp_expr_table_p tab, basic_block bb)
  {
    block_stmt_iterator bsi;
!   tree stmt, def;
    stmt_ann_t ann;
    int partition;
    var_map map = tab->map;
--- 1626,1632 ----
  find_replaceable_in_bb (temp_expr_table_p tab, basic_block bb)
  {
    block_stmt_iterator bsi;
!   tree stmt, def, call;
    stmt_ann_t ann;
    int partition;
    var_map map = tab->map;
*************** find_replaceable_in_bb (temp_expr_table_
*** 1638,1643 ****
--- 1638,1651 ----
        stmt = bsi_stmt (bsi);
        ann = stmt_ann (stmt);
  
+       /* If stmt contains a call in a tail position, do nothing.  There
+ 	 is also no need to kill any expressions in the table because a
+ 	 call in a tail position should also be the last statement in
+ 	 the basic block.  */
+       call = get_call_expr_in (stmt);
+       if (call && CALL_EXPR_TAILCALL (call))
+ 	continue;
+ 
        /* Determine if this stmt finishes an existing expression.  */
        FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_USE)
  	{
Index: tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree.h,v
retrieving revision 1.679
diff -c -3 -p -r1.679 tree.h
*** tree.h	16 Jan 2005 15:28:15 -0000	1.679
--- tree.h	25 Jan 2005 18:59:52 -0000
*************** extern void tree_operand_check_failed (i
*** 824,829 ****
--- 824,832 ----
     had its address taken.  That matters for inline functions.  */
  #define TREE_ADDRESSABLE(NODE) ((NODE)->common.addressable_flag)
  
+ /* Set on a CALL_EXPR if the call is in a tail position, ie. just before the
+    exit of a function.  Calls for which this is true are candidates for tail
+    call optimizations.  */
  #define CALL_EXPR_TAILCALL(NODE) (CALL_EXPR_CHECK(NODE)->common.addressable_flag)
  
  /* In a VAR_DECL, nonzero means allocate static storage.