Patchwork [wide-int] small cleanup in wide-int.*

login
register
mail settings
Submitter Kenneth Zadeck
Date Dec. 6, 2013, 6:13 p.m.
Message ID <52A213DE.2050100@naturalbridge.com>
Download mbox | patch
Permalink /patch/298170/
State New
Headers show

Comments

Kenneth Zadeck - Dec. 6, 2013, 6:13 p.m.
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
>>
>

Patch

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