Re: suspect code in fold-const.c
On Fri, 15 Nov 2013, Kenneth Zadeck wrote: This patch fixes a number of places where the mode bitsize had been used but the mode precision should have been used. The tree level is somewhat sloppy about this - some places use the mode precision and some use the mode bitsize. It seems that the mode precision is the proper choice since it does the correct thing if the underlying mode is a partial int mode. This code has been tested on x86-64 with no regressions. Ok to commit? Ok. Thanks, Richard. 2013-11-15 Kenneth Zadeck zad...@naturalbridge.com * tree.c (int_fits_type_p): Change GET_MODE_BITSIZE to GET_MODE_PRECISION. * fold-const.c (fold_single_bit_test_into_sign_test, fold_binary_loc): Change GET_MODE_BITSIZE to GET_MODE_PRECISION. Kenny On 11/15/2013 08:32 AM, Kenneth Zadeck wrote: On 11/15/2013 04:07 AM, Eric Botcazou wrote: this code from fold-const.c starts on line 13811. else if (TREE_INT_CST_HIGH (arg1) == signed_max_hi TREE_INT_CST_LOW (arg1) == signed_max_lo TYPE_UNSIGNED (arg1_type) /* We will flip the signedness of the comparison operator associated with the mode of arg1, so the sign bit is specified by this mode. Check that arg1 is the signed max associated with this sign bit. */ width == GET_MODE_BITSIZE (TYPE_MODE (arg1_type)) /* signed_type does not work on pointer types. */ INTEGRAL_TYPE_P (arg1_type)) with width defined as: unsigned int width = TYPE_PRECISION (arg1_type); it seems that the check on bitsize should really be a check on the precision of the variable. If this seems right, i will correct this on the trunk and make the appropriate changes to the wide-int branch. Do you mean width == GET_MODE_PRECISION (TYPE_MODE (arg1_type)) instead? If so, that would probably make sense, but there are a few other places with the same TYPE_PRECISION/GET_MODE_BITSIZE check, in particular the very similar transformation done in fold_single_bit_test_into_sign_test. yes. I understand the need to do this check on the mode rather than the precision of the type itself. The point is that if the mode under this type happens to be a partial int mode, then that sign bit may not even be where the bitsize points to. However, having just done a few greps, it looks like this case was just the one that i found while doing the wide-int work, there may be several more of these cases. Just in fold-const, there are a couple in fold_binary_loc. The one in tree.c:int_fits_type_p looks particularly wrong. I think that there are also several in tree-vect-patterns.c. Kenny -- Richard Biener rguent...@suse.de SUSE / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 GF: Jeff Hawn, Jennifer Guild, Felix Imendorffer
Re: suspect code in fold-const.c
committed as revision 204987.
thanks
kenny
On 11/18/2013 05:38 AM, Richard Biener wrote:
On Fri, 15 Nov 2013, Kenneth Zadeck wrote:
This patch fixes a number of places where the mode bitsize had been used but
the mode precision should have been used. The tree level is somewhat sloppy
about this - some places use the mode precision and some use the mode bitsize.
It seems that the mode precision is the proper choice since it does the
correct thing if the underlying mode is a partial int mode.
This code has been tested on x86-64 with no regressions. Ok to commit?
Ok.
Thanks,
Richard.
2013-11-15 Kenneth Zadeck zad...@naturalbridge.com
* tree.c (int_fits_type_p): Change GET_MODE_BITSIZE to
GET_MODE_PRECISION.
* fold-const.c (fold_single_bit_test_into_sign_test,
fold_binary_loc): Change GET_MODE_BITSIZE to
GET_MODE_PRECISION.
Kenny
On 11/15/2013 08:32 AM, Kenneth Zadeck wrote:
On 11/15/2013 04:07 AM, Eric Botcazou wrote:
this code from fold-const.c starts on line 13811.
else if (TREE_INT_CST_HIGH (arg1) == signed_max_hi
TREE_INT_CST_LOW (arg1) == signed_max_lo
TYPE_UNSIGNED (arg1_type)
/* We will flip the signedness of the comparison operator
associated with the mode of arg1, so the sign bit is
specified by this mode. Check that arg1 is the signed
max associated with this sign bit. */
width == GET_MODE_BITSIZE (TYPE_MODE (arg1_type))
/* signed_type does not work on pointer types. */
INTEGRAL_TYPE_P (arg1_type))
with width defined as:
unsigned int width = TYPE_PRECISION (arg1_type);
it seems that the check on bitsize should really be a check on the
precision of the variable. If this seems right, i will correct this on
the trunk and make the appropriate changes to the wide-int branch.
Do you mean
width == GET_MODE_PRECISION (TYPE_MODE (arg1_type))
instead? If so, that would probably make sense, but there are a few other
places with the same TYPE_PRECISION/GET_MODE_BITSIZE check, in particular
the
very similar transformation done in fold_single_bit_test_into_sign_test.
yes. I understand the need to do this check on the mode rather than the
precision of the type itself.
The point is that if the mode under this type happens to be a partial int
mode, then that sign bit may not even be where the bitsize points to.
However, having just done a few greps, it looks like this case was just the
one that i found while doing the wide-int work, there may be several more of
these cases. Just in fold-const, there are a couple in fold_binary_loc.
The one in tree.c:int_fits_type_p looks particularly wrong.
I think that there are also several in tree-vect-patterns.c.
Kenny
Index: gcc/tree.c
===
--- gcc/tree.c (revision 204986)
+++ gcc/tree.c (working copy)
@@ -8629,7 +8629,7 @@ retry:
/* Third, unsigned integers with top bit set never fit signed types. */
if (! TYPE_UNSIGNED (type) unsc)
{
- int prec = GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (c))) - 1;
+ int prec = GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (c))) - 1;
if (prec HOST_BITS_PER_WIDE_INT)
{
if (unsigned HOST_WIDE_INT) 1) prec) dc.low) != 0)
Index: gcc/ChangeLog
===
--- gcc/ChangeLog (revision 204986)
+++ gcc/ChangeLog (working copy)
@@ -1,3 +1,11 @@
+2013-11-18 Kenneth Zadeck zad...@naturalbridge.com
+
+ * tree.c (int_fits_type_p): Change GET_MODE_BITSIZE to
+ GET_MODE_PRECISION.
+ * fold-const.c (fold_single_bit_test_into_sign_test)
+ (fold_binary_loc): Change GET_MODE_BITSIZE to
+ GET_MODE_PRECISION.
+
2013-11-18 Teresa Johnson tejohn...@google.com
* gcc/cfgrtl.c (cfg_layout_initialize): Assert if we
Index: gcc/fold-const.c
===
--- gcc/fold-const.c (revision 204986)
+++ gcc/fold-const.c (working copy)
@@ -6593,7 +6593,7 @@ fold_single_bit_test_into_sign_test (loc
/* This is only a win if casting to a signed type is cheap,
i.e. when arg00's type is not a partial mode. */
TYPE_PRECISION (TREE_TYPE (arg00))
- == GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (arg00
+ == GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (arg00
{
tree stype = signed_type_for (TREE_TYPE (arg00));
return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR,
@@ -12049,7 +12049,7 @@ fold_binary_loc (location_t loc,
zerobits = unsigned HOST_WIDE_INT) 1) shiftc) - 1);
else if (TREE_CODE (arg0) == RSHIFT_EXPR
TYPE_PRECISION (TREE_TYPE (arg0))
- == GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (arg0
+ == GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (arg0
{
prec = TYPE_PRECISION (TREE_TYPE (arg0));
tree arg00 = TREE_OPERAND (arg0, 0);
@@
Re: suspect code in fold-const.c
On 11/15/2013 04:07 AM, Eric Botcazou wrote: this code from fold-const.c starts on line 13811. else if (TREE_INT_CST_HIGH (arg1) == signed_max_hi TREE_INT_CST_LOW (arg1) == signed_max_lo TYPE_UNSIGNED (arg1_type) /* We will flip the signedness of the comparison operator associated with the mode of arg1, so the sign bit is specified by this mode. Check that arg1 is the signed max associated with this sign bit. */ width == GET_MODE_BITSIZE (TYPE_MODE (arg1_type)) /* signed_type does not work on pointer types. */ INTEGRAL_TYPE_P (arg1_type)) with width defined as: unsigned int width = TYPE_PRECISION (arg1_type); it seems that the check on bitsize should really be a check on the precision of the variable. If this seems right, i will correct this on the trunk and make the appropriate changes to the wide-int branch. Do you mean width == GET_MODE_PRECISION (TYPE_MODE (arg1_type)) instead? If so, that would probably make sense, but there are a few other places with the same TYPE_PRECISION/GET_MODE_BITSIZE check, in particular the very similar transformation done in fold_single_bit_test_into_sign_test. yes. I understand the need to do this check on the mode rather than the precision of the type itself. The point is that if the mode under this type happens to be a partial int mode, then that sign bit may not even be where the bitsize points to. However, having just done a few greps, it looks like this case was just the one that i found while doing the wide-int work, there may be several more of these cases. Just in fold-const, there are a couple in fold_binary_loc. The one in tree.c:int_fits_type_p looks particularly wrong. I think that there are also several in tree-vect-patterns.c. Kenny
Re: suspect code in fold-const.c
This patch fixes a number of places where the mode bitsize had been used
but the mode precision should have been used. The tree level is
somewhat sloppy about this - some places use the mode precision and some
use the mode bitsize. It seems that the mode precision is the proper
choice since it does the correct thing if the underlying mode is a
partial int mode.
This code has been tested on x86-64 with no regressions. Ok to commit?
2013-11-15 Kenneth Zadeck zad...@naturalbridge.com
* tree.c (int_fits_type_p): Change GET_MODE_BITSIZE to
GET_MODE_PRECISION.
* fold-const.c (fold_single_bit_test_into_sign_test,
fold_binary_loc): Change GET_MODE_BITSIZE to
GET_MODE_PRECISION.
Kenny
On 11/15/2013 08:32 AM, Kenneth Zadeck wrote:
On 11/15/2013 04:07 AM, Eric Botcazou wrote:
this code from fold-const.c starts on line 13811.
else if (TREE_INT_CST_HIGH (arg1) == signed_max_hi
TREE_INT_CST_LOW (arg1) == signed_max_lo
TYPE_UNSIGNED (arg1_type)
/* We will flip the signedness of the comparison
operator
associated with the mode of arg1, so the sign bit is
specified by this mode. Check that arg1 is the signed
max associated with this sign bit. */
width == GET_MODE_BITSIZE (TYPE_MODE (arg1_type))
/* signed_type does not work on pointer types. */
INTEGRAL_TYPE_P (arg1_type))
with width defined as:
unsigned int width = TYPE_PRECISION (arg1_type);
it seems that the check on bitsize should really be a check on the
precision of the variable. If this seems right, i will correct
this on
the trunk and make the appropriate changes to the wide-int branch.
Do you mean
width == GET_MODE_PRECISION (TYPE_MODE (arg1_type))
instead? If so, that would probably make sense, but there are a few
other
places with the same TYPE_PRECISION/GET_MODE_BITSIZE check, in
particular the
very similar transformation done in fold_single_bit_test_into_sign_test.
yes. I understand the need to do this check on the mode rather than
the precision of the type itself.
The point is that if the mode under this type happens to be a partial
int mode, then that sign bit may not even be where the bitsize points to.
However, having just done a few greps, it looks like this case was
just the one that i found while doing the wide-int work, there may be
several more of these cases. Just in fold-const, there are a couple
in fold_binary_loc. The one in tree.c:int_fits_type_p looks
particularly wrong.
I think that there are also several in tree-vect-patterns.c.
Kenny
Index: gcc/fold-const.c
===
--- gcc/fold-const.c (revision 204842)
+++ gcc/fold-const.c (working copy)
@@ -6593,7 +6593,7 @@ fold_single_bit_test_into_sign_test (loc
/* This is only a win if casting to a signed type is cheap,
i.e. when arg00's type is not a partial mode. */
TYPE_PRECISION (TREE_TYPE (arg00))
- == GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (arg00
+ == GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (arg00
{
tree stype = signed_type_for (TREE_TYPE (arg00));
return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR,
@@ -12050,7 +12050,7 @@ fold_binary_loc (location_t loc,
zerobits = unsigned HOST_WIDE_INT) 1) shiftc) - 1);
else if (TREE_CODE (arg0) == RSHIFT_EXPR
TYPE_PRECISION (TREE_TYPE (arg0))
- == GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (arg0
+ == GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (arg0
{
prec = TYPE_PRECISION (TREE_TYPE (arg0));
tree arg00 = TREE_OPERAND (arg0, 0);
@@ -12061,7 +12061,7 @@ fold_binary_loc (location_t loc,
{
tree inner_type = TREE_TYPE (TREE_OPERAND (arg00, 0));
if (TYPE_PRECISION (inner_type)
- == GET_MODE_BITSIZE (TYPE_MODE (inner_type))
+ == GET_MODE_PRECISION (TYPE_MODE (inner_type))
TYPE_PRECISION (inner_type) prec)
{
prec = TYPE_PRECISION (inner_type);
@@ -13816,7 +13816,7 @@ fold_binary_loc (location_t loc,
associated with the mode of arg1, so the sign bit is
specified by this mode. Check that arg1 is the signed
max associated with this sign bit. */
- width == GET_MODE_BITSIZE (TYPE_MODE (arg1_type))
+ width == GET_MODE_PRECISION (TYPE_MODE (arg1_type))
/* signed_type does not work on pointer types. */
INTEGRAL_TYPE_P (arg1_type))
{
Index: gcc/tree.c
===
--- gcc/tree.c (revision 204842)
+++ gcc/tree.c (working copy)
@@ -8614,7 +8614,7 @@ retry:
/* Third, unsigned integers with top bit set never fit signed types. */
if (! TYPE_UNSIGNED (type) unsc)
{
- int prec = GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (c))) - 1;
+ int prec = GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (c))) - 1;
Re: suspect code in fold-const.c
On 11/14/13 09:16, Kenneth Zadeck wrote: in doing the wide int conversion, i have found the following code on the trunk which seems somewhat suspect: this code from fold-const.c starts on line 13811. else if (TREE_INT_CST_HIGH (arg1) == signed_max_hi TREE_INT_CST_LOW (arg1) == signed_max_lo TYPE_UNSIGNED (arg1_type) /* We will flip the signedness of the comparison operator associated with the mode of arg1, so the sign bit is specified by this mode. Check that arg1 is the signed max associated with this sign bit. */ width == GET_MODE_BITSIZE (TYPE_MODE (arg1_type)) /* signed_type does not work on pointer types. */ INTEGRAL_TYPE_P (arg1_type)) it seems that the check on bitsize should really be a check on the precision of the variable. If this seems right, i will correct this on the trunk and make the appropriate changes to the wide-int branch. I'd strongly suggest talking with Eric. It looks like the mode test is on purpose. From the change which introduced that code: commit 8aa01816af90c5c9b6e3718ee263678ce5f3d93e Author: ebotcazou ebotcazou@138bc75d-0d04-0410-961f-82ee72b054a4 Date: Fri Dec 1 22:46:45 2006 + * fold-const.c (fold_binary) LT_EXPR: Use the precision of the type instead of the size of its mode to compute the highest and lowest possible values. Still check the size of the mode before flipping the signedness of the comparison. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@119422 138bc75d-0d04-0410-96 And it's associated thread on gcc-patches: http://gcc.gnu.org/ml/gcc-patches/2006-11/msg00242.html
