Patchwork [wide-int] resolve bootstrap issue

login
register
mail settings
Submitter Richard Sandiford
Date Jan. 16, 2014, 10:55 a.m.
Message ID <87ppns5h7j.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com>
Download mbox | patch
Permalink /patch/311692/
State New
Headers show

Comments

Richard Sandiford - Jan. 16, 2014, 10:55 a.m.
Mike Stump <mikestump@comcast.net> writes:
> On Jan 14, 2014, at 7:25 AM, Richard Sandiford
> <rsandifo@linux.vnet.ibm.com> wrote:
>> Mike Stump <mikestump@comcast.net> writes:
>>> diff --git a/gcc/expmed.c b/gcc/expmed.c
>>> index ce063eb..720d8c1 100644
>>> --- a/gcc/expmed.c
>>> +++ b/gcc/expmed.c
>>> @@ -4963,6 +4963,7 @@ make_tree (tree type, rtx x)
>>>       return t;
>>> 
>>>     case CONST_DOUBLE:
>>> +      gcc_assert (HOST_BITS_PER_WIDE_INT * 2 <= MAX_BITSIZE_MODE_ANY_INT);
>>>       if (TARGET_SUPPORTS_WIDE_INT == 0 && GET_MODE (x) == VOIDmode)
>>> 	t = wide_int_to_tree (type,
>>> 			      wide_int::from_array (&CONST_DOUBLE_LOW (x), 2,
>> 
>> I think this would be better as a STATIC_ASSERT.
>
> The cost should be the same in generated code.  The C folks can read and
> understand the gcc_assert easier, and the static property of the check
> we don't make any use of.  I could change it, but there are 0 of these
> in *.c presently, and I think we want to tread lightly on those folks
> that know and like C, and don't want gcc to be `different' for no good
> reason.  That said, if people think it would be good to use it, I'd be
> happy to change it.

Static asserts are better than run-time asserts because they're caught
earlier, at GCC compile time.  Yes, wide-int is the first thing to make
much use of it, but something has to be first.

>>> @@ -1440,10 +1442,10 @@ real_to_integer (const REAL_VALUE_TYPE *r, bool *fail, int precision)
>>> 	}
>>> #endif
>>>       w = SIGSZ * HOST_BITS_PER_LONG + words * HOST_BITS_PER_WIDE_INT;
>>> -      result = wide_int::from_array
>>> +      tmp = real_int::from_array
>>> 	(val, (w + HOST_BITS_PER_WIDE_INT - 1) / HOST_BITS_PER_WIDE_INT, w);
>>> -      result = wi::lrshift (result, (words * HOST_BITS_PER_WIDE_INT) - exp);
>>> -      result = wide_int::from (result, precision, UNSIGNED);
>>> +      tmp = wi::lrshift<real_int> (tmp, (words * HOST_BITS_PER_WIDE_INT) - exp);
>>> +      result = wide_int::from (tmp, precision, UNSIGNED);
>> 
>> Why did you need the <real_int>?  It was supposed to work without.
>
> The code in question needs something that is max int + max significand
> real in size, we made the max int smaller (smaller than this quantity on
> x86) so, this code needs a special wide int that is bigger.  The type is
> free as vrp uses the same type.  As for why Kenny choose this method,
> I'd defer to him.

To be clear, I was only talking about the <real_int> in
"wi::lrshift<real_int>".  Just "wi::lrshift" should be fine.

Tested on x86_64-linux-gnu.  OK to install?

Thanks,
Richard
Mike Stump - Jan. 17, 2014, 8:29 p.m.
On Jan 16, 2014, at 2:55 AM, Richard Sandiford <rsandifo@linux.vnet.ibm.com> wrote:
>>> Why did you need the <real_int>?  It was supposed to work without.
>> 
>> The code in question needs something that is max int + max significand
>> real in size, we made the max int smaller (smaller than this quantity on
>> x86) so, this code needs a special wide int that is bigger.  The type is
>> free as vrp uses the same type.  As for why Kenny choose this method,
>> I'd defer to him.
> 
> To be clear, I was only talking about the <real_int> in
> "wi::lrshift<real_int>".  Just "wi::lrshift" should be fine.
> 
> Tested on x86_64-linux-gnu.  OK to install?

Ah, yes, I was trying to get it to compile at one point and added that; I now see what you mean. Yes, this is fine.

> Index: gcc/real.c
> ===================================================================
> --- gcc/real.c	2014-01-15 16:39:39.883276568 +0000
> +++ gcc/real.c	2014-01-15 16:39:40.376274546 +0000
> @@ -1444,7 +1444,7 @@ real_to_integer (const REAL_VALUE_TYPE *
>       w = SIGSZ * HOST_BITS_PER_LONG + words * HOST_BITS_PER_WIDE_INT;
>       tmp = real_int::from_array
> 	(val, (w + HOST_BITS_PER_WIDE_INT - 1) / HOST_BITS_PER_WIDE_INT, w);
> -      tmp = wi::lrshift<real_int> (tmp, (words * HOST_BITS_PER_WIDE_INT) - exp);
> +      tmp = wi::lrshift (tmp, (words * HOST_BITS_PER_WIDE_INT) - exp);
>       result = wide_int::from (tmp, precision, UNSIGNED);
> 
>       if (r->sign)

Patch

Index: gcc/expmed.c
===================================================================
--- gcc/expmed.c	2014-01-15 16:39:39.881276576 +0000
+++ gcc/expmed.c	2014-01-15 16:39:40.375274550 +0000
@@ -4963,7 +4963,7 @@  make_tree (tree type, rtx x)
       return t;
 
     case CONST_DOUBLE:
-      gcc_assert (HOST_BITS_PER_WIDE_INT * 2 <= MAX_BITSIZE_MODE_ANY_INT);
+      STATIC_ASSERT (HOST_BITS_PER_WIDE_INT * 2 <= MAX_BITSIZE_MODE_ANY_INT);
       if (TARGET_SUPPORTS_WIDE_INT == 0 && GET_MODE (x) == VOIDmode)
 	t = wide_int_to_tree (type,
 			      wide_int::from_array (&CONST_DOUBLE_LOW (x), 2,
Index: gcc/real.c
===================================================================
--- gcc/real.c	2014-01-15 16:39:39.883276568 +0000
+++ gcc/real.c	2014-01-15 16:39:40.376274546 +0000
@@ -1444,7 +1444,7 @@  real_to_integer (const REAL_VALUE_TYPE *
       w = SIGSZ * HOST_BITS_PER_LONG + words * HOST_BITS_PER_WIDE_INT;
       tmp = real_int::from_array
 	(val, (w + HOST_BITS_PER_WIDE_INT - 1) / HOST_BITS_PER_WIDE_INT, w);
-      tmp = wi::lrshift<real_int> (tmp, (words * HOST_BITS_PER_WIDE_INT) - exp);
+      tmp = wi::lrshift (tmp, (words * HOST_BITS_PER_WIDE_INT) - exp);
       result = wide_int::from (tmp, precision, UNSIGNED);
 
       if (r->sign)