[PATCH,i386] Fix adxintrin on mingw.

Discussion:

Ilya Tocar

2014-10-02 14:29:46 UTC

Hi,

sizeof (long) == 4 on windows, so we should use long long as param type.
Patch below does it.
Ok for trunk?

2014-10-02 Ilya Tocar <***@intel.com>

* config/i386/adxintrin.h (_subborrow_u64): Use long long for param
type.
(_addcarry_u64): Ditto.
(_addcarryx_u64): Ditto.

---
gcc/config/i386/adxintrin.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/gcc/config/i386/adxintrin.h b/gcc/config/i386/adxintrin.h
index 8f2c01a..00a9b86 100644
--- a/gcc/config/i386/adxintrin.h
+++ b/gcc/config/i386/adxintrin.h
@@ -55,24 +55,24 @@ _addcarryx_u32 (unsigned char __CF, unsigned int __X,
#ifdef __x86_64__
extern __inline unsigned char
__attribute__((__gnu_inline__, __always_inline__, __artificial__))
-_subborrow_u64 (unsigned char __CF, unsigned long __X,
- unsigned long __Y, unsigned long long *__P)
+_subborrow_u64 (unsigned char __CF, unsigned long long __X,
+ unsigned long long __Y, unsigned long long *__P)
{
return __builtin_ia32_sbb_u64 (__CF, __Y, __X, __P);
}

extern __inline unsigned char
__attribute__((__gnu_inline__, __always_inline__, __artificial__))
-_addcarry_u64 (unsigned char __CF, unsigned long __X,
- unsigned long __Y, unsigned long long *__P)
+_addcarry_u64 (unsigned char __CF, unsigned long long __X,
+ unsigned long long __Y, unsigned long long *__P)
{
return __builtin_ia32_addcarryx_u64 (__CF, __X, __Y, __P);
}

extern __inline unsigned char
__attribute__((__gnu_inline__, __always_inline__, __artificial__))
-_addcarryx_u64 (unsigned char __CF, unsigned long __X,
- unsigned long __Y, unsigned long long *__P)
+_addcarryx_u64 (unsigned char __CF, unsigned long long __X,
+ unsigned long long __Y, unsigned long long *__P)
{
return __builtin_ia32_addcarryx_u64 (__CF, __X, __Y, __P);
}

--
1.8.3.1

H.J. Lu

2014-10-02 14:41:34 UTC

Permalink

Post by Ilya Tocar
Hi,
sizeof (long) == 4 on windows, so we should use long long as param type.
Patch below does it.

The same is true for x32. Can you add a testcase to show it
fails on x32 without the fix?

Post by Ilya Tocar
Ok for trunk?
* config/i386/adxintrin.h (_subborrow_u64): Use long long for param
type.
(_addcarry_u64): Ditto.
(_addcarryx_u64): Ditto.
---
gcc/config/i386/adxintrin.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/gcc/config/i386/adxintrin.h b/gcc/config/i386/adxintrin.h
index 8f2c01a..00a9b86 100644
--- a/gcc/config/i386/adxintrin.h
+++ b/gcc/config/i386/adxintrin.h
@@ -55,24 +55,24 @@ _addcarryx_u32 (unsigned char __CF, unsigned int __X,
#ifdef __x86_64__
extern __inline unsigned char
__attribute__((__gnu_inline__, __always_inline__, __artificial__))
-_subborrow_u64 (unsigned char __CF, unsigned long __X,
- unsigned long __Y, unsigned long long *__P)
+_subborrow_u64 (unsigned char __CF, unsigned long long __X,
+ unsigned long long __Y, unsigned long long *__P)
{
return __builtin_ia32_sbb_u64 (__CF, __Y, __X, __P);
}
extern __inline unsigned char
__attribute__((__gnu_inline__, __always_inline__, __artificial__))
-_addcarry_u64 (unsigned char __CF, unsigned long __X,
- unsigned long __Y, unsigned long long *__P)
+_addcarry_u64 (unsigned char __CF, unsigned long long __X,
+ unsigned long long __Y, unsigned long long *__P)
{
return __builtin_ia32_addcarryx_u64 (__CF, __X, __Y, __P);
}
extern __inline unsigned char
__attribute__((__gnu_inline__, __always_inline__, __artificial__))
-_addcarryx_u64 (unsigned char __CF, unsigned long __X,
- unsigned long __Y, unsigned long long *__P)
+_addcarryx_u64 (unsigned char __CF, unsigned long long __X,
+ unsigned long long __Y, unsigned long long *__P)
{
return __builtin_ia32_addcarryx_u64 (__CF, __X, __Y, __P);
}
--
1.8.3.1

--
H.J.

Ilya Tocar

2014-10-03 13:46:36 UTC

Permalink

Post by H.J. Lu

Post by Ilya Tocar
Hi,
sizeof (long) == 4 on windows, so we should use long long as param type.
Patch below does it.

The same is true for x32. Can you add a testcase to show it
fails on x32 without the fix?

This could only be done with runtime test.
I've had troubles running sde (emulator) on x32 enabled system,
but replacing long long with int in intrinsic signature will cause
adx-addcarryx64-2.c to fail under sde on 64 bits. I believe it will
also fail on sde+{win,x32} or real hardware, when it's available.

Post by H.J. Lu

--
H.J.

H.J. Lu

2014-10-03 14:53:21 UTC

Permalink

Post by Ilya Tocar

Post by H.J. Lu

Post by Ilya Tocar
Hi,
sizeof (long) == 4 on windows, so we should use long long as param type.
Patch below does it.

The same is true for x32. Can you add a testcase to show it
fails on x32 without the fix?

Can we scan the assembly output for the wrong instruction?

--
H.J.

Ilya Tocar

2014-10-06 11:16:13 UTC

Permalink

Post by H.J. Lu

Post by Ilya Tocar

Post by H.J. Lu
The same is true for x32. Can you add a testcase to show it
fails on x32 without the fix?

Can we scan the assembly output for the wrong instruction?

I don't think so.
Incorrect instruction is movl (instead of movq) for value load.
However with x32 we also generate movl for pointer moves,
So scanning for movl doesn't make sense. And hardcoding particular
movl will make test quite fragile.

Uros Bizjak

2014-10-06 11:59:37 UTC

Permalink

Post by Ilya Tocar
Hi,
sizeof (long) == 4 on windows, so we should use long long as param type.
Patch below does it.
Ok for trunk?
* config/i386/adxintrin.h (_subborrow_u64): Use long long for param
type.
(_addcarry_u64): Ditto.
(_addcarryx_u64): Ditto.

OK everywhere.

Thanks,
Uros.