preexpand_calls vs. TARGET_EXPRs

Mark Mitchell mark@codesourcery.com
Sun Oct 22 21:42:00 GMT 2000


>>>>> "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 ----


More information about the Gcc-bugs mailing list