Re: [PATCH] Fix ACATS failures in GCC 3.4

[Eric Botcazou <ebotcazou at act-europe dot fr>]

> Hi Eric,

Hi Roger (and sorry for taking so much of your time these days),

> The rule for MIN_EXPR and MAX_EXPR is (should be?) that the tree type of
> their operands should/must match that of the operator.  However, I agree
> that it is better defensive programming to also modify the RTL expanders
> to consistently take the unsignedp flag from the operator's type.  Hence
> if it ever encounters "suspect" trees in future, it will atleast behave
> consistently.  To paraphrase Richard Henderson: "Why fix a bug only once
> when it can be fixed twice?".

OK.

> The sweeping under the rug argument is the wrong way around.  Only
> tweaking the MIN_EXPR and MAX_EXPR RTL expanders would be papering over
> the bug in the constant folder.  However, I'll preapprove a patch that
> does both.

Thanks.  Here is the 3.4 version of the patch, it fixes the ACATS failures.  
OK for mainline and 3.4 branch (after a complete testing cycle)?

2004-02-26  Eric Botcazou  <ebotcazou@act-europe.fr>
            Roger Sayle  <roger@eyesopen.com>

        * fold-const.c (fold): Revert 2004-02-25 change.  Use the original
	operands to build a tree with swapped operands.
	* expr.c (expand_expr_real) <MAX_EXPR>: Consistently use the
	'unsignedp' predicate to specify the signedness.


> As mentioned by Richard Kenner, we should probably also remove the
> RSHIFT case if there are no testsuite regressions, but this is certainly
> more risky and probably not appropriate for the 3.4 branch.  If you
> prefer we can leave your existing workaround on the branch, and cure
> the underlying problems swapping/unsignedp/RSHIFT on mainline.

Workarounds I like not :-)

-- 
Eric Botcazou

Index: fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.322.2.5
diff -u -p -r1.322.2.5 fold-const.c
--- fold-const.c	25 Feb 2004 18:20:44 -0000	1.322.2.5
+++ fold-const.c	26 Feb 2004 16:19:06 -0000
@@ -5377,10 +5377,7 @@ fold (tree expr)
 	  if (op == 0)
 	    continue;		/* Valid for CALL_EXPR, at least.  */
 
-	  if (kind == '<'
-	      || code == MAX_EXPR
-	      || code == MIN_EXPR
-	      || code == RSHIFT_EXPR)
+	  if (kind == '<' || code == RSHIFT_EXPR)
 	    {
 	      /* Signedness matters here.  Perhaps we can refine this
 		 later.  */
@@ -5415,7 +5412,7 @@ fold (tree expr)
        || code == MAX_EXPR || code == BIT_IOR_EXPR || code == BIT_XOR_EXPR
        || code == BIT_AND_EXPR)
       && tree_swap_operands_p (arg0, arg1, true))
-    return fold (build (code, type, arg1, arg0));
+    return fold (build (code, type, TREE_OPERAND (t, 1), TREE_OPERAND (t, 0)));
 
   /* Now WINS is set as described above,
      ARG0 is the first operand of EXPR,
Index: expr.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.c,v
retrieving revision 1.615.4.4
diff -u -p -r1.615.4.4 expr.c
--- expr.c	19 Feb 2004 07:56:23 -0000	1.615.4.4
+++ expr.c	26 Feb 2004 16:19:39 -0000
@@ -8020,7 +8023,7 @@ expand_expr_real (tree exp, rtx target, 
       /* First try to do it with a special MIN or MAX instruction.
 	 If that does not win, use a conditional jump to select the proper
 	 value.  */
-      this_optab = (TREE_UNSIGNED (type)
+      this_optab = (unsignedp
 		    ? (code == MIN_EXPR ? umin_optab : umax_optab)
 		    : (code == MIN_EXPR ? smin_optab : smax_optab));
 
@@ -8054,18 +8057,16 @@ expand_expr_real (tree exp, rtx target, 
 	  && ! can_compare_p (GE, mode, ccp_jump))
 	{
 	  if (code == MAX_EXPR)
-	    do_jump_by_parts_greater_rtx (mode, TREE_UNSIGNED (type),
-					  target, op1, NULL_RTX, op0);
+	    do_jump_by_parts_greater_rtx (mode, unsignedp, target, op1,
+					  NULL_RTX, op0);
 	  else
-	    do_jump_by_parts_greater_rtx (mode, TREE_UNSIGNED (type),
-					  op1, target, NULL_RTX, op0);
+	    do_jump_by_parts_greater_rtx (mode, unsignedp, op1, target,
+					  NULL_RTX, op0);
 	}
       else
 	{
-	  int unsignedp = TREE_UNSIGNED (TREE_TYPE (TREE_OPERAND (exp, 1)));
 	  do_compare_rtx_and_jump (target, op1, code == MAX_EXPR ? GE : LE,
-				   unsignedp, mode, NULL_RTX, NULL_RTX,
-				   op0);
+				   unsignedp, mode, NULL_RTX, NULL_RTX, op0);
 	}
       emit_move_insn (target, op1);
       emit_label (op0);