This is the mail archive of the
gcc-bugs@gcc.gnu.org
mailing list for the GCC project.
Re: preexpand_calls vs. TARGET_EXPRs
>>>>> "Geoff" == Geoff Keating <geoffk@cygnus.com> writes:
Geoff> Mark Mitchell <mark@codesourcery.com> writes:
>> (f () + 1) + (g (), 8)
>>
>> we pre-evaluate the calls to `f' and `g' before expanding the
>> PLUS_EXPR.
>>
>> I'm not sure exactly what the point of that it is. It doesn't
>> seem to be for correctness (unless there's something about the
>> ever-troublesome do_pending_stack_adjust -- and in that case
>> there's probably still a bug since preexpand_calls doesn't
>> expand builtins, even though they can sometimes result in
>> actualy function calls). Therefore, I assume it's some attempt
>> at optimization. (If so, it's probably misguided; we should be
>> depending on real algorithms to reorder code, not playing games
>> during tree-expansion.)
Geoff> I expect much of the time it's a de-optimisation. For
Geoff> instance, given a() + b() + c(), if you evaluate it as
...
Geoff> So if there are no correctness problems---and IMO any
Geoff> correctness problems should be fixed in another way than by
Geoff> doing this---I'd recommend taking it out.
I thought about this more, and I agree completely. There doesn't seem
to be any correctness issue, and like you say: if there is, that's
probably a problem with something else.
So, I've nuked the code, which fixes the C++ test-case (attached).
Tested on i686-pc-linux-gnu.
Thanks for your help!
--
Mark Mitchell mark@codesourcery.com
CodeSourcery, LLC http://www.codesourcery.com
2000-10-22 Mark Mitchell <mark@codesourcery.com>
* expr.c (do_preexpand_calls): Remove.
(same_from_p): Don't use CALL_EXPR_RTL.
(expand_expr): Don't call preexpand_calls, or use CALL_EXPR_RTL.
(preexpand_calls): Remove.
* tree.c (first_rtl_op): Remove CALL_EXPR case.
(unsave_expr_1): Likewise.
* tree.def (CALL_EXPR): Give it only two slots.
* tree.h (CALL_EXPR_RTL): Remove.
2000-10-22 Mark Mitchell <mark@codesourcery.com>
* optimize.c (copy_body_r): Don't treat CALL_EXPRs specially.
Index: gcc/testsuite/g++.old-deja/g++.other/dtor10.C
===================================================================
RCS file: dtor10.C
diff -N dtor10.C
*** /dev/null Tue May 5 13:32:27 1998
--- dtor10.C Sun Oct 22 21:35:52 2000
***************
*** 0 ****
--- 1,40 ----
+ // Origin: Mark Mitchell <mark@codesourcery.com>
+
+ extern "C" void abort ();
+
+ int j;
+
+ struct S {
+ static S* s[5];
+
+ S () { s[j++] = this; }
+ S (const S&) { s[j++] = this; }
+ ~S () {
+ for (int k = 0; k < j; ++k)
+ if (s[k] == this)
+ return;
+ abort ();
+ }
+ };
+
+ S* S::s[5];
+
+ struct T {
+ int i;
+ S s;
+ };
+
+ T t;
+
+ T f () {
+ return t;
+ }
+
+ void g (S) {
+ };
+
+ int main ()
+ {
+ g (f ().s);
+ }
+
Index: gcc/expr.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/expr.c,v
retrieving revision 1.270
diff -c -p -r1.270 expr.c
*** expr.c 2000/10/20 20:57:21 1.270
--- expr.c 2000/10/23 04:35:43
*************** int (*lang_safe_from_p) PARAMS ((rtx, tr
*** 102,112 ****
the same indirect address eventually. */
int cse_not_expected;
- /* Nonzero to generate code for all the subroutines within an
- expression before generating the upper levels of the expression.
- Nowadays this is never zero. */
- int do_preexpand_calls = 1;
-
/* Don't check memory usage, since code is being emitted to check a memory
usage. Used when current_function_check_memory_usage is true, to avoid
infinite recursion. */
--- 102,107 ----
*************** static rtx var_rtx PARAMS ((tree));
*** 183,189 ****
static int readonly_fields_p PARAMS ((tree));
static rtx expand_expr_unaligned PARAMS ((tree, unsigned int *));
static rtx expand_increment PARAMS ((tree, int, int));
- static void preexpand_calls PARAMS ((tree));
static void do_jump_by_parts_greater PARAMS ((tree, int, rtx, rtx));
static void do_jump_by_parts_equality PARAMS ((tree, rtx, rtx));
static void do_compare_and_jump PARAMS ((tree, enum rtx_code, enum rtx_code,
--- 178,183 ----
*************** safe_from_p (x, exp, top_p)
*** 5533,5548 ****
break;
case CALL_EXPR:
! exp_rtl = CALL_EXPR_RTL (exp);
! if (exp_rtl == 0)
! {
! /* Assume that the call will clobber all hard registers and
! all of memory. */
! if ((GET_CODE (x) == REG && REGNO (x) < FIRST_PSEUDO_REGISTER)
! || GET_CODE (x) == MEM)
! return 0;
! }
!
break;
case RTL_EXPR:
--- 5527,5537 ----
break;
case CALL_EXPR:
! /* Assume that the call will clobber all hard registers and
! all of memory. */
! if ((GET_CODE (x) == REG && REGNO (x) < FIRST_PSEUDO_REGISTER)
! || GET_CODE (x) == MEM)
! return 0;
break;
case RTL_EXPR:
*************** expand_expr (exp, target, tmode, modifie
*** 7030,7037 ****
rtx rlow;
rtx diff, quo, rem, addr, bit, result;
- preexpand_calls (exp);
-
/* If domain is empty, answer is no. Likewise if index is constant
and out of bounds. */
if (((TREE_CODE (set_high_bound) == INTEGER_CST
--- 7019,7024 ----
*************** expand_expr (exp, target, tmode, modifie
*** 7154,7164 ****
return expand_builtin (exp, target, subtarget, tmode, ignore);
}
- /* If this call was expanded already by preexpand_calls,
- just return the result we got. */
- if (CALL_EXPR_RTL (exp) != 0)
- return CALL_EXPR_RTL (exp);
-
return expand_call (exp, target, ignore);
case NON_LVALUE_EXPR:
--- 7141,7146 ----
*************** expand_expr (exp, target, tmode, modifie
*** 7354,7360 ****
|| mode != ptr_mode)
goto binop;
- preexpand_calls (exp);
if (! safe_from_p (subtarget, TREE_OPERAND (exp, 1), 1))
subtarget = 0;
--- 7336,7341 ----
*************** expand_expr (exp, target, tmode, modifie
*** 7453,7459 ****
goto binop;
case MULT_EXPR:
- preexpand_calls (exp);
/* If first operand is constant, swap them.
Thus the following special case checks need only
check the second operand. */
--- 7434,7439 ----
*************** expand_expr (exp, target, tmode, modifie
*** 7580,7586 ****
case CEIL_DIV_EXPR:
case ROUND_DIV_EXPR:
case EXACT_DIV_EXPR:
- preexpand_calls (exp);
if (! safe_from_p (subtarget, TREE_OPERAND (exp, 1), 1))
subtarget = 0;
/* Possible optimization: compute the dividend with EXPAND_SUM
--- 7560,7565 ----
*************** expand_expr (exp, target, tmode, modifie
*** 7598,7604 ****
case FLOOR_MOD_EXPR:
case CEIL_MOD_EXPR:
case ROUND_MOD_EXPR:
- preexpand_calls (exp);
if (! safe_from_p (subtarget, TREE_OPERAND (exp, 1), 1))
subtarget = 0;
op0 = expand_expr (TREE_OPERAND (exp, 0), subtarget, VOIDmode, 0);
--- 7577,7582 ----
*************** expand_expr (exp, target, tmode, modifie
*** 7760,7766 ****
case RSHIFT_EXPR:
case LROTATE_EXPR:
case RROTATE_EXPR:
- preexpand_calls (exp);
if (! safe_from_p (subtarget, TREE_OPERAND (exp, 1), 1))
subtarget = 0;
op0 = expand_expr (TREE_OPERAND (exp, 0), subtarget, VOIDmode, 0);
--- 7738,7743 ----
*************** expand_expr (exp, target, tmode, modifie
*** 7782,7788 ****
case UNGT_EXPR:
case UNGE_EXPR:
case UNEQ_EXPR:
- preexpand_calls (exp);
temp = do_store_flag (exp, target, tmode != VOIDmode ? tmode : mode, 0);
if (temp != 0)
return temp;
--- 7759,7764 ----
*************** expand_expr (exp, target, tmode, modifie
*** 8280,8286 ****
&& TREE_CODE (lhs) != PARM_DECL
&& ! (TREE_CODE (lhs) == INDIRECT_REF
&& TYPE_READONLY (TREE_TYPE (TREE_OPERAND (lhs, 0)))))
- preexpand_calls (exp);
/* Check for |= or &= of a bitfield of size one into another bitfield
of size 1. In this case, (unless we need the result of the
--- 8256,8261 ----
*************** expand_expr (exp, target, tmode, modifie
*** 8608,8614 ****
/* Here to do an ordinary binary operator, generating an instruction
from the optab already placed in `this_optab'. */
binop:
- preexpand_calls (exp);
if (! safe_from_p (subtarget, TREE_OPERAND (exp, 1), 1))
subtarget = 0;
op0 = expand_expr (TREE_OPERAND (exp, 0), subtarget, VOIDmode, 0);
--- 8583,8588 ----
*************** expand_increment (exp, post, ignore)
*** 9180,9267 ****
return temp;
}
- /* Expand all function calls contained within EXP, innermost ones first.
- But don't look within expressions that have sequence points.
- For each CALL_EXPR, record the rtx for its value
- in the CALL_EXPR_RTL field. */
-
- static void
- preexpand_calls (exp)
- tree exp;
- {
- register int nops, i;
- int class = TREE_CODE_CLASS (TREE_CODE (exp));
-
- if (! do_preexpand_calls)
- return;
-
- /* Only expressions and references can contain calls. */
-
- if (! IS_EXPR_CODE_CLASS (class) && class != 'r')
- return;
-
- switch (TREE_CODE (exp))
- {
- case CALL_EXPR:
- /* Do nothing if already expanded. */
- if (CALL_EXPR_RTL (exp) != 0
- /* Do nothing if the call returns a variable-sized object. */
- || (TREE_CODE (TREE_TYPE (exp)) != VOID_TYPE
- && TREE_CODE (TYPE_SIZE (TREE_TYPE (exp))) != INTEGER_CST)
- /* Do nothing to built-in functions. */
- || (TREE_CODE (TREE_OPERAND (exp, 0)) == ADDR_EXPR
- && (TREE_CODE (TREE_OPERAND (TREE_OPERAND (exp, 0), 0))
- == FUNCTION_DECL)
- && DECL_BUILT_IN (TREE_OPERAND (TREE_OPERAND (exp, 0), 0))))
- return;
-
- CALL_EXPR_RTL (exp) = expand_call (exp, NULL_RTX, 0);
- return;
-
- case COMPOUND_EXPR:
- case COND_EXPR:
- case TRUTH_ANDIF_EXPR:
- case TRUTH_ORIF_EXPR:
- /* If we find one of these, then we can be sure
- the adjust will be done for it (since it makes jumps).
- Do it now, so that if this is inside an argument
- of a function, we don't get the stack adjustment
- after some other args have already been pushed. */
- do_pending_stack_adjust ();
- return;
-
- case BLOCK:
- case RTL_EXPR:
- case WITH_CLEANUP_EXPR:
- case CLEANUP_POINT_EXPR:
- case TRY_CATCH_EXPR:
- return;
-
- case SAVE_EXPR:
- if (SAVE_EXPR_RTL (exp) != 0)
- return;
-
- default:
- break;
- }
-
- nops = TREE_CODE_LENGTH (TREE_CODE (exp));
- for (i = 0; i < nops; i++)
- if (TREE_OPERAND (exp, i) != 0)
- {
- if (TREE_CODE (exp) == TARGET_EXPR && i == 2)
- /* We don't need to preexpand the cleanup for a TARGET_EXPR.
- It doesn't happen before the call is made. */
- ;
- else
- {
- class = TREE_CODE_CLASS (TREE_CODE (TREE_OPERAND (exp, i)));
- if (IS_EXPR_CODE_CLASS (class) || class == 'r')
- preexpand_calls (TREE_OPERAND (exp, i));
- }
- }
- }
-
/* At the start of a function, record that we have no previously-pushed
arguments waiting to be popped. */
--- 9154,9159 ----
*************** do_store_flag (exp, target, mode, only_c
*** 10428,10434 ****
return 0;
}
- preexpand_calls (exp);
if (! get_subtarget (target)
|| GET_MODE (subtarget) != operand_mode
|| ! safe_from_p (subtarget, arg1, 1))
--- 10320,10325 ----
Index: gcc/tree.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/tree.c,v
retrieving revision 1.165
diff -c -p -r1.165 tree.c
*** tree.c 2000/10/20 20:57:21 1.165
--- tree.c 2000/10/23 04:35:46
*************** first_rtl_op (code)
*** 1764,1771 ****
case GOTO_SUBROUTINE_EXPR:
case RTL_EXPR:
return 0;
- case CALL_EXPR:
- return 2;
case WITH_CLEANUP_EXPR:
/* Should be defined to be 2. */
return 1;
--- 1764,1769 ----
*************** unsave_expr_1 (expr)
*** 1806,1815 ****
/* I don't yet know how to emit a sequence multiple times. */
if (RTL_EXPR_SEQUENCE (expr) != 0)
abort ();
- break;
-
- case CALL_EXPR:
- CALL_EXPR_RTL (expr) = 0;
break;
default:
--- 1804,1809 ----
Index: gcc/tree.def
===================================================================
RCS file: /cvs/gcc/egcs/gcc/tree.def,v
retrieving revision 1.32
diff -c -p -r1.32 tree.def
*** tree.def 2000/06/24 19:26:42 1.32
--- tree.def 2000/10/23 04:35:47
*************** DEFTREECODE (BIND_EXPR, "bind_expr", 'e'
*** 454,463 ****
/* Function call. Operand 0 is the function.
Operand 1 is the argument list, a list of expressions
! made out of a chain of TREE_LIST nodes.
! There is no operand 2. That slot is used for the
! CALL_EXPR_RTL macro (see preexpand_calls). */
! DEFTREECODE (CALL_EXPR, "call_expr", 'e', 3)
/* Call a method. Operand 0 is the method, whose type is a METHOD_TYPE.
Operand 1 is the expression for "self".
--- 454,461 ----
/* Function call. Operand 0 is the function.
Operand 1 is the argument list, a list of expressions
! made out of a chain of TREE_LIST nodes. */
! DEFTREECODE (CALL_EXPR, "call_expr", 'e', 2)
/* Call a method. Operand 0 is the method, whose type is a METHOD_TYPE.
Operand 1 is the expression for "self".
Index: gcc/tree.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/tree.h,v
retrieving revision 1.206
diff -c -p -r1.206 tree.h
*** tree.h 2000/10/22 17:50:26 1.206
--- tree.h 2000/10/23 04:35:49
*************** struct tree_vec
*** 780,788 ****
#define RTL_EXPR_SEQUENCE(NODE) (*(struct rtx_def **) &EXPR_CHECK (NODE)->exp.operands[0])
#define RTL_EXPR_RTL(NODE) (*(struct rtx_def **) &EXPR_CHECK (NODE)->exp.operands[1])
- /* In a CALL_EXPR node. */
- #define CALL_EXPR_RTL(NODE) (*(struct rtx_def **) &EXPR_CHECK (NODE)->exp.operands[2])
-
/* In a CONSTRUCTOR node. */
#define CONSTRUCTOR_ELTS(NODE) TREE_OPERAND (NODE, 1)
--- 780,785 ----
Index: gcc/cp/optimize.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/cp/optimize.c,v
retrieving revision 1.45
diff -c -p -r1.45 optimize.c
*** optimize.c 2000/09/17 07:38:20 1.45
--- optimize.c 2000/10/23 04:35:50
*************** copy_body_r (tp, walk_subtrees, data)
*** 336,345 ****
TREE_OPERAND (*tp, 1) = TREE_OPERAND (*tp, 3);
TREE_OPERAND (*tp, 3) = NULL_TREE;
}
- /* Similarly, if we're copying a CALL_EXPR, the RTL for the
- result is no longer valid. */
- else if (TREE_CODE (*tp) == CALL_EXPR)
- CALL_EXPR_RTL (*tp) = NULL_RTX;
}
/* Keep iterating. */
--- 336,341 ----