This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
sibcall argument evaluation fix
- To: gcc-patches at gcc dot gnu dot org
- Subject: sibcall argument evaluation fix
- From: Richard Henderson <rth at cygnus dot com>
- Date: Fri, 24 Mar 2000 17:20:08 -0800
The following addresses the problem Jason brought up the other day:
http://gcc.gnu.org/ml/gcc-patches/2000-03/msg00782.html
Namely, that the UNSAVE_EXPRs that we were adding for sibcall support
was breaking the SAVE_EXPRs used in exception handling.
I solve this by pre-evaluating such problematic expressions and
saving the result in a temporary VAR_DECL. I'd considered just
not supporting these, but it turned out to be sinfully easy.
This bootstraped and did not regress testsuite on alphaev6-linux
and i686-linux. There are still regressions between no-args and
-O2 in the g++ testsuite, but none introduced by this patch.
r~
* tree.c (lang_safe_for_unsave): Remove.
(unsafe_for_reeval): Transmute and rename from safe_for_unsave,
allowing for two levels of unsafeness. Remove lang hook.
* tree.h: Update declarations.
* calls.c (expand_call): Rename safe_for_reeval to try_tail_call.
Create temporary VAR_DECLs to protect very unsafe_for_reeval trees.
Always fail sibcalls when there are pending cleanups.
Index: calls.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/calls.c,v
retrieving revision 1.103
diff -c -p -d -r1.103 calls.c
*** calls.c 2000/03/24 21:48:00 1.103
--- calls.c 2000/03/25 01:00:42
*************** expand_call (exp, target, ignore)
*** 1696,1702 ****
rtx before_call;
#endif
rtx insn;
! int safe_for_reeval;
int pass;
/* Register in which non-BLKmode value will be returned,
--- 1696,1702 ----
rtx before_call;
#endif
rtx insn;
! int try_tail_call;
int pass;
/* Register in which non-BLKmode value will be returned,
*************** expand_call (exp, target, ignore)
*** 2027,2070 ****
currently_expanding_call++;
! /* If we're considering tail recursion optimizations, verify that the
! arguments are safe for re-evaluation. If we can unsave them, wrap
! each argument in an UNSAVE_EXPR. */
! safe_for_reeval = 0;
if (optimize >= 2
&& currently_expanding_call == 1
&& stmt_loop_nest_empty ()
&& ! any_pending_cleanups (1))
{
! /* Verify that each argument is safe for re-evaluation. */
for (p = actparms; p; p = TREE_CHAIN (p))
! if (! safe_for_unsave (TREE_VALUE (p)))
! break;
! if (p == NULL_TREE)
! {
! tree new_actparms = NULL_TREE, q;
! for (p = actparms; p ; p = TREE_CHAIN (p))
{
! tree np = build_tree_list (TREE_PURPOSE (p),
! unsave_expr (TREE_VALUE (p)));
! if (new_actparms)
! TREE_CHAIN (q) = np;
! else
! new_actparms = np;
! q = np;
}
! actparms = new_actparms;
! safe_for_reeval = 1;
! }
}
/* Generate a tail recursion sequence when calling ourselves. */
! if (safe_for_reeval
&& TREE_CODE (TREE_OPERAND (exp, 0)) == ADDR_EXPR
&& TREE_OPERAND (TREE_OPERAND (exp, 0), 0) == current_function_decl)
{
--- 2027,2096 ----
currently_expanding_call++;
! /* Tail calls can make things harder to debug, and we're traditionally
! pushed these optimizations into -O2. Don't try if we're already
! expanding a call, as that means we're an argument. Similarly, if
! there's pending loops or cleanups we know there's code to follow
! the call. */
! try_tail_call = 0;
if (optimize >= 2
&& currently_expanding_call == 1
&& stmt_loop_nest_empty ()
&& ! any_pending_cleanups (1))
{
! tree new_actparms = NULL_TREE;
!
! /* Ok, we're going to give the tail call the old college try.
! This means we're going to evaluate the function arguments
! up to three times. There are two degrees of badness we can
! encounter, those that can be unsaved and those that can't.
! (See unsafe_for_reeval commentary for details.)
!
! Generate a new argument list. Pass safe arguments through
! unchanged. For the easy badness wrap them in UNSAVE_EXPRs.
! For hard badness, evaluate them now and put their resulting
! rtx in a temporary VAR_DECL. */
!
for (p = actparms; p; p = TREE_CHAIN (p))
! switch (unsafe_for_reeval (TREE_VALUE (p)))
! {
! case 0: /* Safe. */
! new_actparms = tree_cons (TREE_PURPOSE (p), TREE_VALUE (p),
! new_actparms);
! break;
! case 1: /* Mildly unsafe. */
! new_actparms = tree_cons (TREE_PURPOSE (p),
! unsave_expr (TREE_VALUE (p)),
! new_actparms);
! break;
! case 2: /* Wildly unsafe. */
{
! tree var = build_decl (VAR_DECL, NULL_TREE,
! TREE_TYPE (TREE_VALUE (p)));
! DECL_RTL (var) = expand_expr (TREE_VALUE (p), NULL_RTX,
! VOIDmode, EXPAND_NORMAL);
! new_actparms = tree_cons (TREE_PURPOSE (p), var, new_actparms);
}
+ break;
! default:
! abort ();
! }
!
! /* We built the new argument chain backwards. */
! actparms = nreverse (new_actparms);
!
! /* Expanding one of those dangerous arguments could have added
! cleanups, but otherwise give it a whirl. */
! try_tail_call = ! any_pending_cleanups (1);
}
/* Generate a tail recursion sequence when calling ourselves. */
! if (try_tail_call
&& TREE_CODE (TREE_OPERAND (exp, 0)) == ADDR_EXPR
&& TREE_OPERAND (TREE_OPERAND (exp, 0), 0) == current_function_decl)
{
*************** expand_call (exp, target, ignore)
*** 2149,2155 ****
if (pass == 0)
{
/* Various reasons we can not use a sibling call. */
! if (! safe_for_reeval
#ifdef HAVE_sibcall_epilogue
|| ! HAVE_sibcall_epilogue
#else
--- 2175,2181 ----
if (pass == 0)
{
/* Various reasons we can not use a sibling call. */
! if (! try_tail_call
#ifdef HAVE_sibcall_epilogue
|| ! HAVE_sibcall_epilogue
#else
*************** expand_call (exp, target, ignore)
*** 2163,2168 ****
--- 2189,2198 ----
/* If the register holding the address is a callee saved
register, then we lose. We have no way to prevent that,
so we only allow calls to named functions. */
+ /* ??? This could be done by having the insn constraints
+ use a register class that is all call-clobbered. Any
+ reload insns generated to fix things up would appear
+ before the sibcall_epilogue. */
|| fndecl == NULL_TREE
|| ! FUNCTION_OK_FOR_SIBCALL (fndecl))
continue;
*************** expand_call (exp, target, ignore)
*** 2819,2828 ****
/* If there are cleanups to be called, don't use a hard reg as target.
We need to double check this and see if it matters anymore. */
! if (any_pending_cleanups (1)
! && target && REG_P (target)
! && REGNO (target) < FIRST_PSEUDO_REGISTER)
! target = 0, sibcall_failure = 1;
if (TYPE_MODE (TREE_TYPE (exp)) == VOIDmode
|| ignore)
--- 2849,2861 ----
/* If there are cleanups to be called, don't use a hard reg as target.
We need to double check this and see if it matters anymore. */
! if (any_pending_cleanups (1))
! {
! if (target && REG_P (target)
! && REGNO (target) < FIRST_PSEUDO_REGISTER)
! target = 0;
! sibcall_failure = 1;
! }
if (TYPE_MODE (TREE_TYPE (exp)) == VOIDmode
|| ignore)
Index: tree.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/tree.c,v
retrieving revision 1.129
diff -c -p -d -r1.129 tree.c
*** tree.c 2000/03/22 23:55:31 1.129
--- tree.c 2000/03/25 01:00:43
*************** static void print_type_hash_statistics P
*** 290,298 ****
void (*lang_unsave) PARAMS ((tree *));
void (*lang_unsave_expr_now) PARAMS ((tree));
- /* If non-null, a language specific version of safe_for_unsave. */
- int (*lang_safe_for_unsave) PARAMS ((tree));
-
/* The string used as a placeholder instead of a source file name for
built-in tree nodes. The variable, which is dynamically allocated,
should be used; the macro is only used to initialize it. */
--- 290,295 ----
*************** unsave_expr_now (expr)
*** 2675,2689 ****
return expr;
}
! /* Return nonzero if it is safe to unsave EXPR, else return zero.
! It is not safe to unsave EXPR if it contains any embedded RTL_EXPRs. */
int
! safe_for_unsave (expr)
tree expr;
{
enum tree_code code;
! register int i;
int first_rtl;
if (expr == NULL_TREE)
--- 2672,2699 ----
return expr;
}
! /* Return 0 if it is safe to evaluate EXPR multiple times,
! return 1 if it is safe if EXPR is unsaved afterward, or
! return 2 if it is completely unsafe.
!
! This assumes that CALL_EXPRs and TARGET_EXPRs are never replicated in
! an expression tree, so that it safe to unsave them and the surrounding
! context will be correct.
!
! SAVE_EXPRs basically *only* appear replicated in an expression tree,
! occasionally across the whole of a function. It is therefore only
! safe to unsave a SAVE_EXPR if you know that all occurrences appear
! below the UNSAVE_EXPR.
!
! RTL_EXPRs consume their rtl during evaluation. It is therefore
! never possible to unsave them. */
int
! unsafe_for_reeval (expr)
tree expr;
{
enum tree_code code;
! register int i, tmp, unsafeness;
int first_rtl;
if (expr == NULL_TREE)
*************** safe_for_unsave (expr)
*** 2691,2700 ****
code = TREE_CODE (expr);
first_rtl = first_rtl_op (code);
switch (code)
{
case RTL_EXPR:
! return 0;
case CALL_EXPR:
if (TREE_OPERAND (expr, 1)
--- 2701,2713 ----
code = TREE_CODE (expr);
first_rtl = first_rtl_op (code);
+ unsafeness = 0;
+
switch (code)
{
+ case SAVE_EXPR:
case RTL_EXPR:
! return 2;
case CALL_EXPR:
if (TREE_OPERAND (expr, 1)
*************** safe_for_unsave (expr)
*** 2703,2728 ****
tree exp = TREE_OPERAND (expr, 1);
while (exp)
{
! if (! safe_for_unsave (TREE_VALUE (exp)))
! return 0;
exp = TREE_CHAIN (exp);
}
}
break;
default:
! if (lang_safe_for_unsave)
! switch ((*lang_safe_for_unsave) (expr))
! {
! case -1:
! break;
! case 0:
! return 0;
! case 1:
! return 1;
! default:
! abort ();
! }
break;
}
--- 2716,2735 ----
tree exp = TREE_OPERAND (expr, 1);
while (exp)
{
! tmp = unsafe_for_reeval (TREE_VALUE (exp));
! if (tmp > 1)
! return tmp;
exp = TREE_CHAIN (exp);
}
}
+ return 1;
+
+ case TARGET_EXPR:
+ unsafeness = 1;
break;
default:
! /* ??? Add a lang hook if it becomes necessary. */
break;
}
*************** safe_for_unsave (expr)
*** 2733,2739 ****
case 'x': /* something random, like an identifier or an ERROR_MARK. */
case 'd': /* A decl node */
case 'b': /* A block node */
! return 1;
case 'e': /* an expression */
case 'r': /* a reference */
--- 2740,2746 ----
case 'x': /* something random, like an identifier or an ERROR_MARK. */
case 'd': /* A decl node */
case 'b': /* A block node */
! return 0;
case 'e': /* an expression */
case 'r': /* a reference */
*************** safe_for_unsave (expr)
*** 2742,2753 ****
case '2': /* a binary arithmetic expression */
case '1': /* a unary arithmetic expression */
for (i = first_rtl - 1; i >= 0; i--)
! if (! safe_for_unsave (TREE_OPERAND (expr, i)))
! return 0;
! return 1;
default:
! return 0;
}
}
--- 2749,2763 ----
case '2': /* a binary arithmetic expression */
case '1': /* a unary arithmetic expression */
for (i = first_rtl - 1; i >= 0; i--)
! {
! tmp = unsafe_for_reeval (TREE_OPERAND (expr, i));
! if (tmp > unsafeness)
! unsafeness = tmp;
! }
! return unsafeness;
default:
! return 2;
}
}
Index: tree.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/tree.h,v
retrieving revision 1.149
diff -c -p -d -r1.149 tree.h
*** tree.h 2000/03/23 00:20:37 1.149
--- tree.h 2000/03/25 01:00:43
*************** extern tree unsave_expr_now PARAMS ((tr
*** 1963,1974 ****
extern void (*lang_unsave) PARAMS ((tree *));
extern void (*lang_unsave_expr_now) PARAMS ((tree));
! /* If non-null, a language specific version of safe_for_unsave. */
! extern int (*lang_safe_for_unsave) PARAMS ((tree));
!
! /* Return nonzero if it is safe to unsave EXPR, else return zero.
! It is not safe to unsave EXPR if it contains any embedded RTL_EXPRs. */
! extern int safe_for_unsave PARAMS ((tree));
/* Return 1 if EXP contains a PLACEHOLDER_EXPR; i.e., if it represents a size
or offset that depends on a field within a record.
--- 1963,1972 ----
extern void (*lang_unsave) PARAMS ((tree *));
extern void (*lang_unsave_expr_now) PARAMS ((tree));
! /* Return 0 if it is safe to evaluate EXPR multiple times,
! return 1 if it is safe if EXPR is unsaved afterward, or
! return 2 if it is completely unsafe. */
! extern int unsafe_for_reeval PARAMS ((tree));
/* Return 1 if EXP contains a PLACEHOLDER_EXPR; i.e., if it represents a size
or offset that depends on a field within a record.