[Patch] PR 61662

[Patch] PR 61662

[David Wohlferd]

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

As requested, I am posting this patch to gcc-patches.

Problem description:
The detailed description and examples can be found in pr61662, but in 
short: using "#ifdef __x86_64__" to determine the size of a 'long' does 
not reliably yield the correct result.  This causes _lrotl and _lrotr to 
return incorrect results on LLP64 systems (like Windows).

ChangeLog:
2014-07-09  David Wohlferd <dw@LimeGreenSocks.com>

         PR target/61662
         * config/i386/ia32intrin.h: Use __LP64__ to determine size of long

dw

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ia32intrin.h.patch --]
[-- Type: text/x-patch; name="ia32intrin.h.patch", Size: 890 bytes --]

Index: ia32intrin.h
===================================================================
--- ia32intrin.h	(revision 212190)
+++ ia32intrin.h	(working copy)
@@ -256,11 +256,7 @@
 
 #define _bswap64(a)		__bswapq(a)
 #define _popcnt64(a)		__popcntq(a)
-#define _lrotl(a,b)		__rolq((a), (b))
-#define _lrotr(a,b)		__rorq((a), (b))
 #else
-#define _lrotl(a,b)		__rold((a), (b))
-#define _lrotr(a,b)		__rord((a), (b))
 
 /* Read flags register */
 extern __inline unsigned int
@@ -280,6 +276,15 @@
 
 #endif
 
+/* on LP64 systems, longs are 64bits.  Use the appropriate rotate function */
+#ifdef __LP64__
+#define _lrotl(a,b)		__rolq((a), (b))
+#define _lrotr(a,b)		__rorq((a), (b))
+#else
+#define _lrotl(a,b)		__rold((a), (b))
+#define _lrotr(a,b)		__rord((a), (b))
+#endif
+
 #define _bit_scan_forward(a)	__bsfd(a)
 #define _bit_scan_reverse(a)	__bsrd(a)
 #define _bswap(a)		__bswapd(a)

Re: [Patch] PR 61662

[David Wohlferd]

Doh!  Since the component here is 'Target', I probably should have 
included the x86-64 Port Maintainer in the TO line.

Jan, while I have a release on file with the FSF, I don't have SVN write 
access.

dw

On 7/9/2014 12:51 AM, David Wohlferd wrote:
> As requested, I am posting this patch to gcc-patches.
>
> Problem description:
> The detailed description and examples can be found in pr61662, but in 
> short: using "#ifdef __x86_64__" to determine the size of a 'long' 
> does not reliably yield the correct result.  This causes _lrotl and 
> _lrotr to return incorrect results on LLP64 systems (like Windows).
>
> ChangeLog:
> 2014-07-09  David Wohlferd <dw@LimeGreenSocks.com>
>
>         PR target/61662
>         * config/i386/ia32intrin.h: Use __LP64__ to determine size of 
> long
>
> dw

Re: [Patch] PR 61662

[Gerald Pfeifer]

On Wed, 16 Jul 2014, David Wohlferd wrote:
>>> 2014-07-09  David Wohlferd <dw@LimeGreenSocks.com>
>>> 
>>>          PR target/61662
>>>          * config/i386/ia32intrin.h: Use __LP64__ to determine size of
>>> long
>> This is OK for mainline and 4.9 backport.
> Thank you for reviewing and approving this.  However while I have a 
> release on file with FSF, I don't have SVN write permissions, so I 
> cannot check this in.

I applied this to trunk with minor tweaks to the ChangeLog and the 
comment in the patch itself.  (Both should end with a full stop, we 
use 64-bit as an adjective, and the line was getting long.)

Gerald


2014-07-16  David Wohlferd <dw@LimeGreenSocks.com>

	PR target/61662
	* config/i386/ia32intrin.h: Use __LP64__ to determine size of long.

Index: config/i386/ia32intrin.h
===================================================================
--- config/i386/ia32intrin.h	(revision 212698)
+++ config/i386/ia32intrin.h	(working copy)
@@ -256,11 +256,7 @@
 
 #define _bswap64(a)		__bswapq(a)
 #define _popcnt64(a)		__popcntq(a)
-#define _lrotl(a,b)		__rolq((a), (b))
-#define _lrotr(a,b)		__rorq((a), (b))
 #else
-#define _lrotl(a,b)		__rold((a), (b))
-#define _lrotr(a,b)		__rord((a), (b))
 
 /* Read flags register */
 extern __inline unsigned int
@@ -280,6 +276,16 @@
 
 #endif
 
+/* On LP64 systems, longs are 64-bit.  Use the appropriate rotate
+ * function.  */
+#ifdef __LP64__
+#define _lrotl(a,b)		__rolq((a), (b))
+#define _lrotr(a,b)		__rorq((a), (b))
+#else
+#define _lrotl(a,b)		__rold((a), (b))
+#define _lrotr(a,b)		__rord((a), (b))
+#endif
+
 #define _bit_scan_forward(a)	__bsfd(a)
 #define _bit_scan_reverse(a)	__bsrd(a)
 #define _bswap(a)		__bswapd(a)

Re: [Patch] PR 61662

[David Wohlferd]

On 7/15/2014 11:10 AM, Uros Bizjak wrote:
> Hello!
>
>> The detailed description and examples can be found in pr61662, but in short: using "#ifdef
>> __x86_64__" to determine the size of a 'long' does not reliably yield the correct result. This
>> causes _lrotl and _lrotr to return incorrect results on LLP64 systems (like Windows).
>>
>> ChangeLog:
>> 2014-07-09  David Wohlferd <dw@LimeGreenSocks.com>
>>
>>          PR target/61662
>>          * config/i386/ia32intrin.h: Use __LP64__ to determine size of long
> This is OK for mainline and 4.9 backport.

Thank you for reviewing and approving this.  However while I have a 
release on file with FSF, I don't have SVN write permissions, so I 
cannot check this in.

dw

Re: [Patch] PR 61662

[Uros Bizjak]

Hello!

> The detailed description and examples can be found in pr61662, but in short: using "#ifdef
> __x86_64__" to determine the size of a 'long' does not reliably yield the correct result. This
> causes _lrotl and _lrotr to return incorrect results on LLP64 systems (like Windows).
>
> ChangeLog:
> 2014-07-09  David Wohlferd <dw@LimeGreenSocks.com>
>
>         PR target/61662
>         * config/i386/ia32intrin.h: Use __LP64__ to determine size of long

This is OK for mainline and 4.9 backport.

Thanks,
Uros.