| Message ID | 20141002142946.GB54444@msticlxl7.ims.intel.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 2, 2014 at 7:29 AM, Ilya Tocar <tocarip.intel@gmail.com> wrote: > 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? > Ok for trunk? > > 2014-10-02 Ilya Tocar <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 >
On 02 Oct 07:41, H.J. Lu wrote: > On Thu, Oct 2, 2014 at 7:29 AM, Ilya Tocar <tocarip.intel@gmail.com> wrote: > > 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. > > Ok for trunk? > > > > 2014-10-02 Ilya Tocar <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.
On Fri, Oct 3, 2014 at 6:46 AM, Ilya Tocar <tocarip.intel@gmail.com> wrote: > On 02 Oct 07:41, H.J. Lu wrote: >> On Thu, Oct 2, 2014 at 7:29 AM, Ilya Tocar <tocarip.intel@gmail.com> wrote: >> > 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. > Can we scan the assembly output for the wrong instruction?
On 03 Oct 07:53, H.J. Lu wrote: > On Fri, Oct 3, 2014 at 6:46 AM, Ilya Tocar <tocarip.intel@gmail.com> wrote: > > On 02 Oct 07:41, H.J. Lu wrote: > >> On Thu, Oct 2, 2014 at 7:29 AM, Ilya Tocar <tocarip.intel@gmail.com> wrote: > >> > >> 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. > > > > 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.
On Thu, Oct 2, 2014 at 4:29 PM, Ilya Tocar <tocarip.intel@gmail.com> wrote: > 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 <ilya.tocar@intel.com> > > * config/i386/adxintrin.h (_subborrow_u64): Use long long for param > type. > (_addcarry_u64): Ditto. > (_addcarryx_u64): Ditto. OK everywhere. Thanks, Uros.
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); }