diff mbox

[wide-int] Add fast path for hosts with HWI widening multiplication

Message ID 87eh042fom.fsf@talisman.default
State New
Headers show

Commit Message

Richard Sandiford May 8, 2014, 8:12 p.m. UTC
"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Thu, May 8, 2014 at 12:42 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Thu, May 08, 2014 at 12:34:28PM -0700, H.J. Lu wrote:
>>> On Thu, May 8, 2014 at 12:18 PM, Richard Sandiford
>>> <rdsandiford@googlemail.com> wrote:
>>> > Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>> >> everyone who has a private port will hate you forever.   note that i
>>> >> have 2 of them.
>>> >
>>> > Got any other ideas though?  I suppose if we're prepared to break
>>> > compatibility with whatever the upstream of longlong.h is, we could
>>> > make more use of intN_t and uintN_t.
>>> >
>>> > Having a whitelist of hosts seems like the best fix though.
>>> > I'm not sure the default umul_ppmm is going to be any better
>>> > than not defining it.
>>> >
>>>
>>> Can you add a configure time check if
>>>
>>> typedef unsigned int UTItype __attribute__ ((mode (TI)));
>>>
>>> is supported?
>>
>> Why?  Isn't that #ifdef __SIZEOF_INT128__ ?

Read this just after getting the configure test to work...

> Yes, we can use that.  Will it work?

Seems to.  How does this look?

Thanks,
Richard


gcc/
	* wide-int.cc: Only include longlong.h if W_TYPE_SIZE==32 or
	__SIZEOF_INT128__ is defined.

Comments

H.J. Lu May 8, 2014, 8:48 p.m. UTC | #1
On Thu, May 8, 2014 at 1:12 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>> On Thu, May 8, 2014 at 12:42 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> On Thu, May 08, 2014 at 12:34:28PM -0700, H.J. Lu wrote:
>>>> On Thu, May 8, 2014 at 12:18 PM, Richard Sandiford
>>>> <rdsandiford@googlemail.com> wrote:
>>>> > Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>> >> everyone who has a private port will hate you forever.   note that i
>>>> >> have 2 of them.
>>>> >
>>>> > Got any other ideas though?  I suppose if we're prepared to break
>>>> > compatibility with whatever the upstream of longlong.h is, we could
>>>> > make more use of intN_t and uintN_t.
>>>> >
>>>> > Having a whitelist of hosts seems like the best fix though.
>>>> > I'm not sure the default umul_ppmm is going to be any better
>>>> > than not defining it.
>>>> >
>>>>
>>>> Can you add a configure time check if
>>>>
>>>> typedef unsigned int UTItype __attribute__ ((mode (TI)));
>>>>
>>>> is supported?
>>>
>>> Why?  Isn't that #ifdef __SIZEOF_INT128__ ?
>
> Read this just after getting the configure test to work...
>
>> Yes, we can use that.  Will it work?
>
> Seems to.  How does this look?
>
> Thanks,
> Richard
>
>
> gcc/
>         * wide-int.cc: Only include longlong.h if W_TYPE_SIZE==32 or
>         __SIZEOF_INT128__ is defined.
>
> Index: gcc/wide-int.cc
> ===================================================================
> --- gcc/wide-int.cc     2014-05-08 20:48:25.341583885 +0100
> +++ gcc/wide-int.cc     2014-05-08 21:09:29.324386217 +0100
> @@ -27,18 +27,17 @@ along with GCC; see the file COPYING3.
>  #include "tree.h"
>  #include "dumpfile.h"
>
> -#if GCC_VERSION >= 3000
>  #define W_TYPE_SIZE HOST_BITS_PER_WIDE_INT

Isn't  HOST_BITS_PER_WIDE_INT always 64 now?

> +#if GCC_VERSION >= 3000 && (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__))

W_TYPE_SIZE == 32 is always false and on 32-bit hosts,
__SIZEOF_INT128__ won't be defined.

>  typedef unsigned HOST_HALF_WIDE_INT UHWtype;
>  typedef unsigned HOST_WIDE_INT UWtype;
>  typedef unsigned int UQItype __attribute__ ((mode (QI)));
>  typedef unsigned int USItype __attribute__ ((mode (SI)));
>  typedef unsigned int UDItype __attribute__ ((mode (DI)));
> -typedef unsigned int UTItype __attribute__ ((mode (TI)));
>  #if W_TYPE_SIZE == 32
> -# define UDWtype       UDItype
> -#elif W_TYPE_SIZE == 64
> -# define UDWtype       UTItype
> +typedef unsigned int UDWtype __attribute__ ((mode (DI)));

Can't we use

#ifndef __SIZEOF_INT128__

here?

> +#else
> +typedef unsigned int UDWtype __attribute__ ((mode (TI)));
>  #endif
>  #include "longlong.h"
>  #endif
Richard Sandiford May 8, 2014, 9:04 p.m. UTC | #2
"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Thu, May 8, 2014 at 1:12 PM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>> On Thu, May 8, 2014 at 12:42 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>>> On Thu, May 08, 2014 at 12:34:28PM -0700, H.J. Lu wrote:
>>>>> On Thu, May 8, 2014 at 12:18 PM, Richard Sandiford
>>>>> <rdsandiford@googlemail.com> wrote:
>>>>> > Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>>> >> everyone who has a private port will hate you forever.   note that i
>>>>> >> have 2 of them.
>>>>> >
>>>>> > Got any other ideas though?  I suppose if we're prepared to break
>>>>> > compatibility with whatever the upstream of longlong.h is, we could
>>>>> > make more use of intN_t and uintN_t.
>>>>> >
>>>>> > Having a whitelist of hosts seems like the best fix though.
>>>>> > I'm not sure the default umul_ppmm is going to be any better
>>>>> > than not defining it.
>>>>> >
>>>>>
>>>>> Can you add a configure time check if
>>>>>
>>>>> typedef unsigned int UTItype __attribute__ ((mode (TI)));
>>>>>
>>>>> is supported?
>>>>
>>>> Why?  Isn't that #ifdef __SIZEOF_INT128__ ?
>>
>> Read this just after getting the configure test to work...
>>
>>> Yes, we can use that.  Will it work?
>>
>> Seems to.  How does this look?
>>
>> Thanks,
>> Richard
>>
>>
>> gcc/
>>         * wide-int.cc: Only include longlong.h if W_TYPE_SIZE==32 or
>>         __SIZEOF_INT128__ is defined.
>>
>> Index: gcc/wide-int.cc
>> ===================================================================
>> --- gcc/wide-int.cc     2014-05-08 20:48:25.341583885 +0100
>> +++ gcc/wide-int.cc     2014-05-08 21:09:29.324386217 +0100
>> @@ -27,18 +27,17 @@ along with GCC; see the file COPYING3.
>>  #include "tree.h"
>>  #include "dumpfile.h"
>>
>> -#if GCC_VERSION >= 3000
>>  #define W_TYPE_SIZE HOST_BITS_PER_WIDE_INT
>
> Isn't  HOST_BITS_PER_WIDE_INT always 64 now?

Right, but like I said in reply to Ramana's email, there's no guarantee
that we'll continue to use HOST_WIDE_INT for wide-int.h.  I'd rather
keep this code parameterised on W_TYPE_SIZE rather than hard-code
W_TYPE_SIZE==64.

>> +#if GCC_VERSION >= 3000 && (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__))
>
> W_TYPE_SIZE == 32 is always false and on 32-bit hosts,
> __SIZEOF_INT128__ won't be defined.

Right, but we won't try to use TImode if in future we do revert to using
32-bit types for 32-bit hosts.

Thanks,
Richard
Ramana Radhakrishnan May 9, 2014, 8:34 a.m. UTC | #3
>>> +#if GCC_VERSION >= 3000 && (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__))
>>
>> W_TYPE_SIZE == 32 is always false and on 32-bit hosts,
>> __SIZEOF_INT128__ won't be defined.
>
> Right, but we won't try to use TImode if in future we do revert to using
> 32-bit types for 32-bit hosts.

For now, I've reverted my commit until we agree on the best approach 
here as the fallout has been greater than I expected.

I wasn't looking at email last night or would have done this sooner.

Apologies for the breakage.

Ramana
Richard Henderson May 12, 2014, 6:23 p.m. UTC | #4
On 05/08/2014 01:12 PM, Richard Sandiford wrote:
> 	* wide-int.cc: Only include longlong.h if W_TYPE_SIZE==32 or
> 	__SIZEOF_INT128__ is defined.

FWIW, this looks pretty good to me.


r~
Richard Sandiford May 17, 2014, 8:30 a.m. UTC | #5
Richard Henderson <rth@redhat.com> writes:
> On 05/08/2014 01:12 PM, Richard Sandiford wrote:
>> 	* wide-int.cc: Only include longlong.h if W_TYPE_SIZE==32 or
>> 	__SIZEOF_INT128__ is defined.
>
> FWIW, this looks pretty good to me.

Thanks, belatedly applied.

Richard
diff mbox

Patch

Index: gcc/wide-int.cc
===================================================================
--- gcc/wide-int.cc	2014-05-08 20:48:25.341583885 +0100
+++ gcc/wide-int.cc	2014-05-08 21:09:29.324386217 +0100
@@ -27,18 +27,17 @@  along with GCC; see the file COPYING3.
 #include "tree.h"
 #include "dumpfile.h"
 
-#if GCC_VERSION >= 3000
 #define W_TYPE_SIZE HOST_BITS_PER_WIDE_INT
+#if GCC_VERSION >= 3000 && (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__))
 typedef unsigned HOST_HALF_WIDE_INT UHWtype;
 typedef unsigned HOST_WIDE_INT UWtype;
 typedef unsigned int UQItype __attribute__ ((mode (QI)));
 typedef unsigned int USItype __attribute__ ((mode (SI)));
 typedef unsigned int UDItype __attribute__ ((mode (DI)));
-typedef unsigned int UTItype __attribute__ ((mode (TI)));
 #if W_TYPE_SIZE == 32
-# define UDWtype       UDItype
-#elif W_TYPE_SIZE == 64
-# define UDWtype       UTItype
+typedef unsigned int UDWtype __attribute__ ((mode (DI)));
+#else
+typedef unsigned int UDWtype __attribute__ ((mode (TI)));
 #endif
 #include "longlong.h"
 #endif