This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[patch] Fix for PR c/11449. (take 2)
Hi,
Attached is an updated patch to fix PR c/11449, incorporating
suggestions from Roger Sayle. Consider
int
foo (int m)
{
return !(m & ((int) 0x80000000));
}
fold_single_bit_test() wants to convert "return ...;" to
return m >= 0;
using sign_bit_p(), which determines if 0x80000000 is a sign bit. Due
to a bug that I describe later, sign_bit_p() says no. Then
fold_single_bit_test() goes on to create
return (m >> CST) & 1; or return m >> CST;
The latter is created if we are testing the sign bit. Then because of
"!", invert_truthvalue() is called to invert the truth value of (m >>
31), but invert_truthvalue() does not know how to handle RROTATE_EXPR,
so it calls abort().
This story contains two problems. One is sign_bit_p(). The other is
part of fold_single_bit_test that is run when sign_bit_p() says no.
First, fold_single_bit_test() calls sign_bit_p() to see if the bit
being tested is a sign bit or not. However, sign_bit_p() does not
handle the case where the bit mask is an signed integer. In the above
example, sign_bit_p() looks for
0x00000000 80000000,
but (int) 0x80000000 is represented as
0xffffffff 80000000,
so sign_bit_p() says no. The first two hunks of the patch fixes this
problem by masking off unnecessary part of val, those bits beyond the
32 bits in this case.
When sign_bit_p() says no, fold_single_bit_test() wants to convert
"return ...;" to
return m >> CST;
if we are testing the sign bit, but we have already taken care of that
case, so all we have to create should be
(m >> CST) & 1.
The rest of the patch removes the code that creates m >> CST without
"& 1".
Tested on i686-pc-linux-gnu. OK to apply?
Kazu Hirata
2003-07-07 Kazu Hirata <kazu@cs.umass.edu>
PR c/11449
* fold-const.c (sign_bit_p): Return EXP if VAL is the sign bit
of HOST_WIDE_INT.
(fold_single_bit_test): If sign_bit_p() fails, assume that the
bit being tested is not a sign bit.
2003-07-07 Kazu Hirata <kazu@cs.umass.edu>
PR c/11449
* gcc.c-torture/compile/20030707-1.c: New.
Index: fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.277
diff -c -r1.277 fold-const.c
*** fold-const.c 7 Jul 2003 19:11:55 -0000 1.277
--- fold-const.c 7 Jul 2003 22:14:09 -0000
***************
*** 2719,2726 ****
static tree
sign_bit_p (tree exp, tree val)
{
! unsigned HOST_WIDE_INT lo;
! HOST_WIDE_INT hi;
int width;
tree t;
--- 2719,2726 ----
static tree
sign_bit_p (tree exp, tree val)
{
! unsigned HOST_WIDE_INT mask_lo, lo;
! HOST_WIDE_INT mask_hi, hi;
int width;
tree t;
***************
*** 2739,2752 ****
{
hi = (unsigned HOST_WIDE_INT) 1 << (width - HOST_BITS_PER_WIDE_INT - 1);
lo = 0;
}
else
{
hi = 0;
lo = (unsigned HOST_WIDE_INT) 1 << (width - 1);
}
! if (TREE_INT_CST_HIGH (val) == hi && TREE_INT_CST_LOW (val) == lo)
return exp;
/* Handle extension from a narrower type. */
--- 2739,2763 ----
{
hi = (unsigned HOST_WIDE_INT) 1 << (width - HOST_BITS_PER_WIDE_INT - 1);
lo = 0;
+
+ mask_hi = ((unsigned HOST_WIDE_INT) -1
+ >> (2 * HOST_BITS_PER_WIDE_INT - width));
+ mask_lo = -1;
}
else
{
hi = 0;
lo = (unsigned HOST_WIDE_INT) 1 << (width - 1);
+
+ mask_hi = 0;
+ mask_lo = ((unsigned HOST_WIDE_INT) -1
+ >> (HOST_BITS_PER_WIDE_INT - width));
}
! /* We mask off those bits beyond TREE_TYPE (exp) so that we can
! treat VAL as if it were unsigned. */
! if ((TREE_INT_CST_HIGH (val) & mask_hi) == hi
! && (TREE_INT_CST_LOW (val) & mask_lo) == lo)
return exp;
/* Handle extension from a narrower type. */
***************
*** 4826,4831 ****
--- 4837,4846 ----
convert (stype, arg00),
convert (stype, integer_zero_node)));
}
+
+ /* At this point, we know that arg0 is not testing the sign bit. */
+ if (TYPE_PRECISION (type) - 1 == bitnum)
+ abort ();
/* Otherwise we have (A & C) != 0 where C is a single bit,
convert that into ((A >> C2) & 1). Where C2 = log2(C).
***************
*** 4847,4859 ****
/* If we are going to be able to omit the AND below, we must do our
operations as unsigned. If we must use the AND, we have a choice.
Normally unsigned is faster, but for some machines signed is. */
- ops_unsigned = (bitnum == TYPE_PRECISION (type) - 1 ? 1
#ifdef LOAD_EXTEND_OP
! : (LOAD_EXTEND_OP (operand_mode) == SIGN_EXTEND ? 0 : 1)
#else
! : 1
#endif
- );
signed_type = (*lang_hooks.types.type_for_mode) (operand_mode, 0);
unsigned_type = (*lang_hooks.types.type_for_mode) (operand_mode, 1);
--- 4862,4872 ----
/* If we are going to be able to omit the AND below, we must do our
operations as unsigned. If we must use the AND, we have a choice.
Normally unsigned is faster, but for some machines signed is. */
#ifdef LOAD_EXTEND_OP
! ops_unsigned = (LOAD_EXTEND_OP (operand_mode) == SIGN_EXTEND ? 0 : 1);
#else
! ops_unsigned = 1;
#endif
signed_type = (*lang_hooks.types.type_for_mode) (operand_mode, 0);
unsigned_type = (*lang_hooks.types.type_for_mode) (operand_mode, 1);
***************
*** 4867,4875 ****
inner, integer_one_node);
/* Put the AND last so it can combine with more things. */
! if (bitnum != TYPE_PRECISION (type) - 1)
! inner = build (BIT_AND_EXPR, ops_unsigned ? unsigned_type : signed_type,
! inner, integer_one_node);
/* Make sure to return the proper type. */
if (TREE_TYPE (inner) != result_type)
--- 4880,4888 ----
inner, integer_one_node);
/* Put the AND last so it can combine with more things. */
! inner = build (BIT_AND_EXPR, ops_unsigned ? unsigned_type : signed_type,
! inner, integer_one_node);
/* Make sure to return the proper type. */
if (TREE_TYPE (inner) != result_type)
--- /dev/null 2003-01-30 05:24:37.000000000 -0500
+++ 20030707-1.c 2003-07-07 18:22:23.000000000 -0400
@@ -0,0 +1,7 @@
+/* PR c/11449. */
+
+int
+foo (int m)
+{
+ return !(m & ((int) 0x80000000));
+}