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 PR c/3444


Hi!

The following patch fixes PR c/3444 (simplified in the testcase below).
The problem is that build_binary_op in this case (BIT_XOR_EXPR
with one argument integer_cst -103, the other argument
(int) (unsigned short) c (where c has signed char type)) both sets
shorten = -1
and shortens by hand, because one argument is integer constant, the type
in topmost nop_expr's operand is unsigned and shorter than int.
This shortening consists of:
	  final_type = result_type;
	  op1 = TREE_OPERAND (op1, 0);
	  result_type = TREE_TYPE (op1);
where final_type = result_type is clearly redundant (both are NULL), kills
the topmost nop_expr and sets result_type to its operand type.
But almost immediately after that:
      if (shorten || common || short_compare)
        result_type = common_type (type0, type1);
so the computed result_type is blown away immediately and the only effect is
that op1 now doesn't have int type but unsigned short, so get_narrower
doesn't return unsigned short, but signed char.
To me the code removed in the patch below (ie. shortening in BIT_AND_EXPR,
BIT_XOR_EXPR... case) looks redundant to what setting shorten to -1 does
below (appart from causing this testcase to fail).
I've checked e.g.:
unsigned char c = 0xff; int foo (void) { return (unsigned char) 0x99 ^ c; }
which I think is what the optimization was really targetting (so that the
xor is done in QImode and converted into SImode, not computed in SImode)
and plain setting shorten = -1 did exactly the same job as gcc used to do.
Ok to commit if testing is ok?

2002-02-16  Jakub Jelinek  <jakub@redhat.com>

	PR c/3444:
	* c-typeck.c (build_binary_op) [BIT_XOR_EXPR]: Remove explicit
	shortening.

	* typeck.c (build_binary_op) [BIT_XOR_EXPR]: Remove explicit
	shortening.

	* gcc.c-torture/execute/20020216-1.c: New test.

--- gcc/testsuite/gcc.c-torture/execute/20020216-1.c.jj	Sat Feb 16 22:54:16 2002
+++ gcc/testsuite/gcc.c-torture/execute/20020216-1.c	Sat Feb 16 23:02:00 2002
@@ -0,0 +1,24 @@
+/* PR c/3444
+   This used to fail because bitwise xor was improperly computed in char type
+   and sign extended to int type.  */
+
+extern void abort ();
+extern void exit (int);
+
+signed char c = (signed char) 0xffffffff;
+
+int foo (void)
+{
+  return (unsigned short) c ^ (signed char) 0x99999999;
+}
+
+int main (void)
+{
+  if ((unsigned char) -1 != 0xff
+      || sizeof (short) != 2
+      || sizeof (int) != 4)
+    exit (0);
+  if (foo () != (int) 0xffff0066)
+    abort ();
+  exit (0);
+}
--- gcc/c-typeck.c.jj	Thu Feb 14 11:29:09 2002
+++ gcc/c-typeck.c	Sat Feb 16 22:48:00 2002
@@ -2060,29 +2060,6 @@ build_binary_op (code, orig_op0, orig_op
     case BIT_XOR_EXPR:
       if (code0 == INTEGER_TYPE && code1 == INTEGER_TYPE)
 	shorten = -1;
-      /* If one operand is a constant, and the other is a short type
-	 that has been converted to an int,
-	 really do the work in the short type and then convert the
-	 result to int.  If we are lucky, the constant will be 0 or 1
-	 in the short type, making the entire operation go away.  */
-      if (TREE_CODE (op0) == INTEGER_CST
-	  && TREE_CODE (op1) == NOP_EXPR
-	  && TYPE_PRECISION (type1) > TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op1, 0)))
-	  && TREE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op1, 0))))
-	{
-	  final_type = result_type;
-	  op1 = TREE_OPERAND (op1, 0);
-	  result_type = TREE_TYPE (op1);
-	}
-      if (TREE_CODE (op1) == INTEGER_CST
-	  && TREE_CODE (op0) == NOP_EXPR
-	  && TYPE_PRECISION (type0) > TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))
-	  && TREE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
-	{
-	  final_type = result_type;
-	  op0 = TREE_OPERAND (op0, 0);
-	  result_type = TREE_TYPE (op0);
-	}
       break;
 
     case TRUNC_MOD_EXPR:
--- gcc/cp/typeck.c.jj	Tue Feb 12 16:19:23 2002
+++ gcc/cp/typeck.c	Sat Feb 16 22:48:21 2002
@@ -3514,31 +3514,6 @@ build_binary_op (code, orig_op0, orig_op
     case BIT_XOR_EXPR:
       if (code0 == INTEGER_TYPE && code1 == INTEGER_TYPE)
 	shorten = -1;
-      /* If one operand is a constant, and the other is a short type
-	 that has been converted to an int,
-	 really do the work in the short type and then convert the
-	 result to int.  If we are lucky, the constant will be 0 or 1
-	 in the short type, making the entire operation go away.  */
-      if (TREE_CODE (op0) == INTEGER_CST
-	  && TREE_CODE (op1) == NOP_EXPR
-	  && (TYPE_PRECISION (type1)
-	      > TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op1, 0))))
-	  && TREE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op1, 0))))
-	{
-	  final_type = result_type;
-	  op1 = TREE_OPERAND (op1, 0);
-	  result_type = TREE_TYPE (op1);
-	}
-      if (TREE_CODE (op1) == INTEGER_CST
-	  && TREE_CODE (op0) == NOP_EXPR
-	  && (TYPE_PRECISION (type0)
-	      > TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0))))
-	  && TREE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
-	{
-	  final_type = result_type;
-	  op0 = TREE_OPERAND (op0, 0);
-	  result_type = TREE_TYPE (op0);
-	}
       break;
 
     case TRUNC_MOD_EXPR:

	Jakub


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