This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix evaluation order assumptions in expr.c


There a still a few places in expr.c that make the assumption that
expand_operands will always expand the first operand before the
second, or more accurately that the suggested subtarget is used
to hold op0 during the evaluation on op1.  This assumption may
not hold for too much longer :>

The following patch removes these evaluation order assumptions.
Particularly, reversing the evaluation order of MIN and MAX would
generate incorrect code, breaking bootstrap on x86.  I've also
added a new testcase checking that MIN and MAX are both functional.
There were no new failures in the testsuite during my experiments,
so it looks like we're misssing some testsuite coverage for GCC's
min/max operators.

The following patch has been tested on i686-pc-linux-gnu with a full
"make bootstrap", all languages except treelang, and regression tested
with a top-level "make -k check" with no new failures.  The new
testcase has always passed, but just ensures we don't break anything
in future.

Ok for mainline?


2003-09-28  Roger Sayle  <roger@eyesopen.com>

	* expr.c (expand_expr <PLUS_EXPR>): Let expand_operands call
	safe_from_p for us, once it chooses an evaluation order.
	(expand_expr <MULT_EXPR>): Likewise.
	(expand_expr <MIN_EXPR> <MAX_EXPR>): Likewise.  If expand_operands
	places the second operand in "target", swap the operands.
	(do_store_flag): Let expand_operands call safe_from_p for us.

	* gcc.c-torture/execute/20030928-1.c: New testcase.


Index: expr.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.c,v
retrieving revision 1.588
diff -c -3 -p -r1.588 expr.c
*** expr.c	26 Sep 2003 06:08:48 -0000	1.588
--- expr.c	28 Sep 2003 05:11:06 -0000
*************** expand_expr (tree exp, rtx target, enum
*** 8175,8183 ****
  	    }
  	}

-       if (! safe_from_p (subtarget, TREE_OPERAND (exp, 1), 1))
- 	subtarget = 0;
-
        /* No sense saving up arithmetic to be done
  	 if it's all in the wrong mode to form part of an address.
  	 And force_operand won't know whether to sign-extend or
--- 8175,8180 ----
*************** expand_expr (tree exp, rtx target, enum
*** 8285,8293 ****
  					     TYPE_MODE (TREE_TYPE (exp1))));
  	}

-       if (! safe_from_p (subtarget, TREE_OPERAND (exp, 1), 1))
- 	subtarget = 0;
-
        if (modifier == EXPAND_STACK_PARM)
  	target = 0;

--- 8282,8287 ----
*************** expand_expr (tree exp, rtx target, enum
*** 8466,8472 ****
        target = original_target;
        if (target == 0
  	  || modifier == EXPAND_STACK_PARM
- 	  || ! safe_from_p (target, TREE_OPERAND (exp, 1), 1)
  	  || (GET_CODE (target) == MEM && MEM_VOLATILE_P (target))
  	  || GET_MODE (target) != mode
  	  || (GET_CODE (target) == REG
--- 8460,8465 ----
*************** expand_expr (tree exp, rtx target, enum
*** 8493,8498 ****
--- 8486,8499 ----
        if (GET_CODE (target) == MEM)
  	target = gen_reg_rtx (mode);

+       /* If op1 was placed in target, swap op0 and op1.  */
+       if (target != op0 && target == op1)
+ 	{
+ 	  rtx tem = op0;
+ 	  op0 = op1;
+ 	  op1 = tem;
+ 	}
+
        if (target != op0)
  	emit_move_insn (target, op0);

*************** do_store_flag (tree exp, rtx target, enu
*** 9981,9988 ****
      }

    if (! get_subtarget (target)
!       || GET_MODE (subtarget) != operand_mode
!       || ! safe_from_p (subtarget, arg1, 1))
      subtarget = 0;

    expand_operands (arg0, arg1, subtarget, &op0, &op1, 0);
--- 9982,9988 ----
      }

    if (! get_subtarget (target)
!       || GET_MODE (subtarget) != operand_mode)
      subtarget = 0;

    expand_operands (arg0, arg1, subtarget, &op0, &op1, 0);


/* Check that MAX_EXPR and MIN_EXPR are working properly.  */

#define MAX(X,Y) ((X) > (Y) ? (X) : (Y))
#define MIN(X,Y) ((X) < (Y) ? (X) : (Y))

extern void abort (void);

int main()
{
  int ll_bitsize, ll_bitpos;
  int rl_bitsize, rl_bitpos;
  int end_bit;

  ll_bitpos = 32;  ll_bitsize = 32;
  rl_bitpos = 0;   rl_bitsize = 32;

  end_bit = MAX (ll_bitpos + ll_bitsize, rl_bitpos + rl_bitsize);
  if (end_bit != 64)
    abort ();
  end_bit = MAX (rl_bitpos + rl_bitsize, ll_bitpos + ll_bitsize);
  if (end_bit != 64)
    abort ();
  end_bit = MIN (ll_bitpos + ll_bitsize, rl_bitpos + rl_bitsize);
  if (end_bit != 32)
    abort ();
  end_bit = MIN (rl_bitpos + rl_bitsize, ll_bitpos + ll_bitsize);
  if (end_bit != 32)
    abort ();
  return 0;
}


Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]