This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [wide-int] small cleanup in wide-int.*
- From: Kenneth Zadeck <zadeck at naturalbridge dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Mike Stump <mikestump at comcast dot net>, rsandifo at linux dot vnet dot ibm dot com
- Date: Fri, 06 Dec 2013 13:13:50 -0500
- Subject: Re: [wide-int] small cleanup in wide-int.*
- Authentication-results: sourceware.org; auth=none
- References: <52977927 dot 5010008 at naturalbridge dot com> <CAFiYyc0+GZC6yGQjoOo66srn9-SaiDCJFf0TsVMLdFP_==qTow at mail dot gmail dot com> <529E04A3 dot 9060806 at naturalbridge dot com> <87txepanch dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <52A1FF29 dot 3050909 at naturalbridge dot com>
Richard asked to see the patch where i did the changes the way that he
asked in his email. Doing it the way he wants potentially has advantages
over the way that i did it, but his technique fails for non obvious
reasons. The failure in building is:
g++ -c -g -DIN_GCC -fno-exceptions -fno-rtti
-fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual
-Wno-format -Wmissing-format-attribute -pedantic -Wno-long-long
-Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H
-I. -I. -I../../gccBadMulVrp/gcc -I../../gccBadMulVrp/gcc/.
-I../../gccBadMulVrp/gcc/../include
-I../../gccBadMulVrp/gcc/../libcpp/include
-I../../gccBadMulVrp/gcc/../libdecnumber
-I../../gccBadMulVrp/gcc/../libdecnumber/bid -I../libdecnumber
-I../../gccBadMulVrp/gcc/../libbacktrace -o tree-vrp.o -MT tree-vrp.o
-MMD -MP -MF ./.deps/tree-vrp.TPo ../../gccBadMulVrp/gcc/tree-vrp.c
In file included from ../../gccBadMulVrp/gcc/double-int.h:23:0,
from ../../gccBadMulVrp/gcc/tree-core.h:28,
from ../../gccBadMulVrp/gcc/tree.h:23,
from ../../gccBadMulVrp/gcc/tree-vrp.c:26:
../../gccBadMulVrp/gcc/wide-int.h: In member function
‘generic_wide_int<storage>& generic_wide_int<T>::operator=(const T&)
[with T = generic_wide_int<fixed_wide_int_storage<1152> >, storage =
wi::extended_tree<1152>]’:
../../gccBadMulVrp/gcc/wide-int.h:701:3: instantiated from
‘generic_wide_int<T>& generic_wide_int<T>::operator-=(const T&) [with T
= generic_wide_int<fixed_wide_int_storage<1152> >, storage =
wi::extended_tree<1152>, generic_wide_int<T> =
generic_wide_int<wi::extended_tree<1152> >]’
../../gccBadMulVrp/gcc/tree-vrp.c:2646:13: instantiated from here
../../gccBadMulVrp/gcc/wide-int.h:860:3: error: no matching function for
call to ‘generic_wide_int<wi::extended_tree<1152> >::operator=(const
generic_wide_int<fixed_wide_int_storage<1152> >&)’
../../gccBadMulVrp/gcc/wide-int.h:860:3: note: candidate is:
../../gccBadMulVrp/gcc/tree.h:4529:9: note: wi::extended_tree<1152>&
wi::extended_tree<1152>::operator=(const wi::extended_tree<1152>&)
../../gccBadMulVrp/gcc/tree.h:4529:9: note: no known conversion for
argument 1 from ‘const generic_wide_int<fixed_wide_int_storage<1152> >’
to ‘const wi::extended_tree<1152>&’
make[3]: *** [tree-vrp.o] Error 1
make[3]: Leaving directory `/home/zadeck/gcc/gbbBadMulVrp/gcc'
make[2]: *** [all-stage1-gcc] Error 2
make[2]: Leaving directory `/home/zadeck/gcc/gbbBadMulVrp'
make[1]: *** [stage1-bubble] Error 2
make[1]: Leaving directory `/home/zadeck/gcc/gbbBadMulVrp'
make: *** [all] Error 2
heracles:~/gcc/gbbBadMulVrp(9) cd ../gccBadMulVrp/
On 12/06/2013 11:45 AM, Kenneth Zadeck wrote:
On 12/03/2013 11:52 AM, Richard Sandiford wrote:
Kenneth Zadeck <zadeck@naturalbridge.com> writes:
Index: tree-vrp.c
===================================================================
--- tree-vrp.c (revision 205597)
+++ tree-vrp.c (working copy)
@@ -2611,22 +2611,28 @@ extract_range_from_binary_expr_1 (value_
signop sign = TYPE_SIGN (expr_type);
unsigned int prec = TYPE_PRECISION (expr_type);
- unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
if (range_int_cst_p (&vr0)
&& range_int_cst_p (&vr1)
&& TYPE_OVERFLOW_WRAPS (expr_type))
{
- wide_int sizem1 = wi::mask (prec, false, prec2);
- wide_int size = sizem1 + 1;
+ /* vrp_int is twice as wide as anything that the target
+ supports so it can support a full width multiply. No
+ need to add any more padding for an extra sign bit
+ because that comes with the way that WIDE_INT_MAX_ELTS is
+ defined. */
+ typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2)
+ vrp_int;
+ vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
+ vrp_int size = sizem1 + 1;
/* Extend the values using the sign of the result to PREC2.
From here on out, everthing is just signed math no matter
what the input types were. */
- wide_int min0 = wide_int::from (vr0.min, prec2, sign);
- wide_int max0 = wide_int::from (vr0.max, prec2, sign);
- wide_int min1 = wide_int::from (vr1.min, prec2, sign);
- wide_int max1 = wide_int::from (vr1.max, prec2, sign);
+ vrp_int min0 = wi::to_vrp (vr0.min);
+ vrp_int max0 = wi::to_vrp (vr0.max);
+ vrp_int min1 = wi::to_vrp (vr1.min);
+ vrp_int max1 = wi::to_vrp (vr1.max);
I think we should avoid putting to_vrp in tree.h if vrp_int is only
local to this block. Instead you could have:
typedef generic_wide_int
<wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
...
vrp_int_cst min0 = vr0.min;
vrp_int_cst max0 = vr0.max;
vrp_int_cst min1 = vr1.min;
vrp_int_cst max1 = vr1.max;
i did this in a different way because i had trouble doing it as you
suggested. the short answer is that all of the vrp_int code is now
local to tree-vrp.c which i think was your primary goal
@@ -228,15 +228,16 @@ along with GCC; see the file COPYING3.
#endif
/* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
- early examination of the target's mode file. Thus it is safe that
- some small multiple of this number is easily larger than any number
- that that target could compute. The place in the compiler that
- currently needs the widest ints is the code that determines the
- range of a multiply. This code needs 2n + 2 bits. */
-
+ early examination of the target's mode file. The WIDE_INT_MAX_ELTS
+ can accomodate at least 1 more bit so that unsigned numbers of that
+ mode can be represented. This will accomodate every place in the
+ compiler except for a multiply routine in tree-vrp. That function
+ makes its own arrangements for larger wide-ints. */
I think we should drop the "This will accomodate..." bit, since it'll
soon
get out of date. Maybe something like:
Note that it is still possible to create fixed_wide_ints that have
precisions greater than MAX_BITSIZE_MODE_ANY_INT. This can be useful
when representing a double-width multiplication result, for example. */
done
#define WIDE_INT_MAX_ELTS \
- ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
- / HOST_BITS_PER_WIDE_INT)
+ (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
+ / HOST_BITS_PER_WIDE_INT) + 1)
I think this should be:
(MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact
multiple
of HOST_BITS_PER_WIDE_INT.
we will do this later when some other issues that Eric B raised are
settled.
ok to commit to the branch?
kenny
Looks good to me otherwise FWIW.
You probably already realise this, but for avoidance of doubt, Richard
was also asking that we reduce MAX_BITSIZE_MODE_ANY_INT to 128 on
x86_64,
since that's the largest scalar_mode_supported_p mode.
Thanks,
Richard
Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h (revision 205726)
+++ gcc/wide-int.h (working copy)
@@ -228,15 +228,17 @@ along with GCC; see the file COPYING3.
#endif
/* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
- early examination of the target's mode file. Thus it is safe that
- some small multiple of this number is easily larger than any number
- that that target could compute. The place in the compiler that
- currently needs the widest ints is the code that determines the
- range of a multiply. This code needs 2n + 2 bits. */
-
+ early examination of the target's mode file. The WIDE_INT_MAX_ELTS
+ can accomodate at least 1 more bit so that unsigned numbers of that
+ mode can be represented. Note that it is still possible to create
+ fixed_wide_ints that have precisions greater than
+ MAX_BITSIZE_MODE_ANY_INT. This can be useful when representing a
+ double-width multiplication result, for example. */
#define WIDE_INT_MAX_ELTS \
- ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
- / HOST_BITS_PER_WIDE_INT)
+ (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
+ / HOST_BITS_PER_WIDE_INT) + 1)
+
+#define WIDE_INT_MAX_PRECISION (WIDE_INT_MAX_ELTS * HOST_BITS_PER_WIDE_INT)
/* This is the max size of any pointer on any machine. It does not
seem to be as easy to sniff this out of the machine description as
@@ -294,7 +296,7 @@ struct wide_int_storage;
typedef generic_wide_int <wide_int_storage> wide_int;
typedef FIXED_WIDE_INT (ADDR_MAX_PRECISION) offset_int;
-typedef FIXED_WIDE_INT (MAX_BITSIZE_MODE_ANY_INT) widest_int;
+typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION) widest_int;
template <bool SE>
struct wide_int_ref_storage;
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c (revision 205726)
+++ gcc/tree-vrp.c (working copy)
@@ -2620,23 +2620,24 @@ extract_range_from_binary_expr_1 (value_
signop sign = TYPE_SIGN (expr_type);
unsigned int prec = TYPE_PRECISION (expr_type);
- unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
if (range_int_cst_p (&vr0)
&& range_int_cst_p (&vr1)
&& TYPE_OVERFLOW_WRAPS (expr_type))
{
- wide_int sizem1 = wi::mask (prec, false, prec2);
- wide_int size = sizem1 + 1;
+ typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;
+ typedef generic_wide_int
+ <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
+ vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
+ vrp_int size = sizem1 + 1;
/* Extend the values using the sign of the result to PREC2.
From here on out, everthing is just signed math no matter
what the input types were. */
- wide_int min0 = wide_int::from (vr0.min, prec2, sign);
- wide_int max0 = wide_int::from (vr0.max, prec2, sign);
- wide_int min1 = wide_int::from (vr1.min, prec2, sign);
- wide_int max1 = wide_int::from (vr1.max, prec2, sign);
-
+ vrp_int_cst min0 = vr0.min;
+ vrp_int_cst max0 = vr0.max;
+ vrp_int_cst min1 = vr1.min;
+ vrp_int_cst max1 = vr1.max;
/* Canonicalize the intervals. */
if (sign == UNSIGNED)
{
@@ -2653,17 +2654,17 @@ extract_range_from_binary_expr_1 (value_
}
}
- wide_int prod0 = min0 * min1;
- wide_int prod1 = min0 * max1;
- wide_int prod2 = max0 * min1;
- wide_int prod3 = max0 * max1;
+ vrp_int prod0 = min0 * min1;
+ vrp_int prod1 = min0 * max1;
+ vrp_int prod2 = max0 * min1;
+ vrp_int prod3 = max0 * max1;
/* Sort the 4 products so that min is in prod0 and max is in
prod3. */
/* min0min1 > max0max1 */
if (wi::gts_p (prod0, prod3))
{
- wide_int tmp = prod3;
+ vrp_int tmp = prod3;
prod3 = prod0;
prod0 = tmp;
}
@@ -2671,21 +2672,21 @@ extract_range_from_binary_expr_1 (value_
/* min0max1 > max0min1 */
if (wi::gts_p (prod1, prod2))
{
- wide_int tmp = prod2;
+ vrp_int tmp = prod2;
prod2 = prod1;
prod1 = tmp;
}
if (wi::gts_p (prod0, prod1))
{
- wide_int tmp = prod1;
+ vrp_int tmp = prod1;
prod1 = prod0;
prod0 = tmp;
}
if (wi::gts_p (prod2, prod3))
{
- wide_int tmp = prod3;
+ vrp_int tmp = prod3;
prod3 = prod2;
prod2 = tmp;
}
Index: gcc/tree.h
===================================================================
--- gcc/tree.h (revision 205726)
+++ gcc/tree.h (working copy)
@@ -4549,7 +4549,7 @@ namespace wi
static const unsigned int precision = N;
};
- generic_wide_int <extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+ generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION> >
to_widest (const_tree);
generic_wide_int <extended_tree <ADDR_MAX_PRECISION> > to_offset (const_tree);
@@ -4570,7 +4570,7 @@ wi::int_traits <const_tree>::decompose (
precision);
}
-inline generic_wide_int <wi::extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION> >
wi::to_widest (const_tree t)
{
return t;
@@ -4609,7 +4609,7 @@ wi::extended_tree <N>::get_len () const
{
if (N == ADDR_MAX_PRECISION)
return TREE_INT_CST_OFFSET_NUNITS (m_t);
- else if (N == MAX_BITSIZE_MODE_ANY_INT)
+ else if (N >= WIDE_INT_MAX_PRECISION)
return TREE_INT_CST_EXT_NUNITS (m_t);
else
/* This class is designed to be used for specific output precisions