classification
Title: Intel icc 9.1 does not support __int128_t used by ctypes
Type: compile error Stage: needs patch
Components: ctypes Versions: Python 2.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Alex.Leach, Arfrever, Jens.Timmerman, jared.jennings, loewis, meador.inge, skrah, theller
Priority: normal Keywords: patch

Created on 2008-10-15 20:42 by jared.jennings, last changed 2013-12-17 16:04 by Jens.Timmerman.

Files
File name Uploaded Description Edit
ffi64.c.patch Alex.Leach, 2012-04-13 19:13 Patch for x86/ffi64.c
ffi64.c.patch Alex.Leach, 2012-04-15 12:27 Unified format diff
Messages (24)
msg74813 - (view) Author: jared jennings (jared.jennings) Date: 2008-10-15 20:42
If the Intel 9.1 compilers are used to compile Python 2.6, the following
compiler error results:

/mnt/gpfs/usrpeople/jenninjl/Python-2.6-intel/Modules/_ctypes/libffi/src/x86/ffi64.c(43):
error: identifier "__int128_t" is undefined
    __int128_t sse[MAX_SSE_REGS];
    ^

Forum thread regarding lack of support for __int128_t in icc:
http://software.intel.com/en-us/forums/intel-c-compiler/topic/56652/

Intel C++ Compiler 10.1 for Linux, Intrinsic Reference:
http://softwarecommunity.intel.com/isn/downloads/softwareproducts/pdfs/347603.pdf

The Intrinsic Reference details (pp. 2-4) the __m128 type supported by
the Intel compiler, and restrictions on its use. (Note that the compiler
used was 9.1 but the documentation is from 10.1.)
msg74814 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-10-15 20:51
Why are you reporting this as a bug in Python? Isn't it rather a bug in icc?
msg74842 - (view) Author: jared jennings (jared.jennings) Date: 2008-10-16 18:29
According to §7.1.3 of the C99 standard, the name __int128_t is reserved
for implementation-specific use because it starts with an underscore. So
if gcc defines this type and icc does not, that is not a bug in icc; and
if Python uses this type, it will only certainly build using gcc. I
doubt such a dependency was intended, because it is not documented, and
no error was thrown when the autoconf script detected that a compiler
other than GCC was being used.

C99 standard: http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1124.pdf

Exact-width integer types (e.g. int64_t) are documented in §7.18.1.1 of
the above.
msg74923 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-10-17 16:10
Hmm. Perhaps icc shouldn't define __x86_64__ either, then?

Can you propose a patch to fix that problem?
msg111219 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2010-07-22 21:21
This thread contains a feature request for __int128_t support in icc and
a workaround:

http://software.intel.com/en-us/forums/showthread.php?t=56652
msg158226 - (view) Author: Alex Leach (Alex.Leach) Date: 2012-04-13 19:13
Patch included for Modules/_ctyles/libffi/src/x86/ffi64.c. I've added some include guards around anything necessary to compile with the Intel compiler.
This patch is needed to compile the _ctypes module with icc on current Python releases (just successfully compiled 2.7.3, with patch)..
msg158323 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-04-15 12:14
Can you please re-upload this as a unified diff?
msg158355 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * Date: 2012-04-15 18:59
I suggest to not apply additional patches for Modules/_ctypes/libffi* due to issue #12081. Patches for libffi should be sent to libffi upstream.
msg158373 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2012-04-15 22:11
> I suggest to not apply additional patches for Modules/_ctypes/libffi* 
> due to issue #12081. Patches for libffi should be sent to libffi 
> upstream.

For trunk I agree.  However, it is probably worth considering this patch
for 2.7 and 3.2.

Did anyone check to see if this is already fixed in upstream libffi?
msg158376 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * Date: 2012-04-15 22:31
https://github.com/atgreen/libffi/blob/master/src/x86/ffi64.c contains:

#ifdef __INTEL_COMPILER
#define UINT128 __m128
#else
#define UINT128 __int128_t
#endif
msg158378 - (view) Author: Alex Leach (Alex.Leach) Date: 2012-04-15 22:47
Submitting a working patch upstream would make sense.. Just found, downloaded and tried compiling libffi-3.0.11. The developers have made some changes towards a solution, but compilation fails with the same error:-

libtool: compile:  icc -DHAVE_CONFIG_H -I. -I.. -I. -I../include -Iinclude -I../src -DFFI_BUILDING -g -O3 -ansi_alias -Wall -fexceptions -MT src/x86/ffi64.lo -MD -MP -MF src/x86/.deps/ffi64.Tpo -c ../src/x86/ffi64.c  -fPIC -DPIC -o src/x86/.libs/ffi64.o
../src/x86/ffi64.c(50): error: identifier "__m128" is undefined
    UINT128 sse[MAX_SSE_REGS];


__m128 is defined in Intel's xmmintrin.h though [http://software.intel.com/sites/products/documentation/hpc/compilerpro/en-us/cpp/lin/compiler_c/intref_cls/common/intref_sse_arithmetic.htm].

So I added the necessary include line, which gets a different error:-

libtool: compile:  icc -DHAVE_CONFIG_H -I. -I.. -I. -I../include -Iinclude -I../src -DFFI_BUILDING -g -O3 -ansi_alias -Wall -fexceptions -MT src/x86/ffi64.lo -MD -MP -MF src/x86/.deps/ffi64.Tpo -c ../src/x86/ffi64.c  -fPIC -DPIC -o src/x86/.libs/ffi64.o
../src/x86/ffi64.c(481): error: a value of type "UINT64={unsigned long}" cannot be assigned to an entity of type "__m128"
                  reg_args->sse[ssecount++] = *(UINT64 *) a;
                                            ^

../src/x86/ffi64.c(484): error: a value of type "UINT32={unsigned int}" cannot be assigned to an entity of type "__m128"
                  reg_args->sse[ssecount++] = *(UINT32 *) a;
                                            ^

Regarding my previous patch, I'm not convinced it works actually.. It compiles, and passes the default Python tests, but I get some dodgy behaviour. e.g. when compiling 2.7.3 with --enable-shared, I get a segfault when compiling the gdbm module against libdb4.8-dev. Any specific ways of testing the build?
msg158436 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-04-16 13:14
Changes to ctypes should not affect the gdbm module.

More likely, icc and python just don't work together. It may be a compiler bug in icc (it wouldn't be the first one discovered by Python), or it may be a portability defect in Python (where it wouldn't be the first case, either). In any case, the gdbm issue is likely unrelated.
msg158491 - (view) Author: Alex Leach (Alex.Leach) Date: 2012-04-16 18:15
Thanks for the assurance. I found it strange because compilation only fails when building shared binaries. My static build of Python-2.7.3, with icc seems to work fine, except for this libffi issue.

Turns out I needed dejagnu to run the tests in libffi's `make check`, which is why it passed before when it shouldn't have.
I'll liaise with the libffi authors to try and come up with a working solution.
msg165045 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2012-07-08 23:36
This is still broken after the libffi update (issue15194).  The errors are the same as Alex mentioned when he tested libffi-3.0.11.  The right way to go is to get this fixed in upstream libffi and backport the patch.
msg165579 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-07-16 09:39
I agree with Meador that this should be fixed upstream first.
There's a thread here on libffi-discuss, with a patch but no
conclusive answer:


http://sourceware.org/ml/libffi-discuss/2012/msg00168.html
msg165580 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-07-16 09:53
Ah, forget that: Alex has apparently already tested that patch.
msg165587 - (view) Author: Alex Leach (Alex.Leach) Date: 2012-07-16 10:56
I hadn't tried the `long long` substitution before, but unfortunately I don't 
think that simple fix quite does the trick.

I just checked out libffi from git, and made the following patch:-
#$ diff -u ffi64.c ffi64.c.orig 

--- ffi64.c     2012-07-16 11:38:44.237255615 +0100
+++ ffi64.c.orig        2012-07-16 11:38:34.681045084 +0100
@@ -38,7 +38,7 @@
 #define MAX_SSE_REGS 8

 #ifdef __INTEL_COMPILER
-#define UINT128 long long
+#define UINT128 __m128
 #else
 #define UINT128 __int128_t
 #endif

It compiles fine, but `make check` returns a lot of FAIL's, and a quite a few 
of the tests timeout. e.g.
FAIL: libffi.special/unwindtest.cc  -shared-libgcc -lstdc++ execution test
WARNING: program timed out.

It's probably also worth mentioning that I'm now using icc version 12.1.4

Cheers,
Alex

On Monday 16 Jul 2012 09:53:36 Stefan Krah wrote:
 > Stefan Krah <stefan-usenet@bytereef.org> added the comment:
> 
> Ah, forget that: Alex has apparently already tested that patch.
> 
> ----------
> 
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue4130>
> _______________________________________
msg165660 - (view) Author: Alex Leach (Alex.Leach) Date: 2012-07-16 22:16
I just had a dig around my cpython build dir, and found an ffi64.c I hacked at a 
while back.
I copied the edits over to the latest libffi git revision, rebuilt, and `make 
check` (of libffi) passes all tests. So as far as I can tell the below patch 
works, but it is a hack, and I'm sure it could be improved..

Regards to submitting it upstream, I've just written to the libffi-
discuss/sourceware.org mailing list, sending them the below patch also. So it 
might work, but you know that GPL clause about coming with no warranty? ;)

HTH,
Alex

libffi-git> diff -u src/x86/ffi64.c.orig src/x86/ffi64.c

--- src/x86/ffi64.c.orig        2012-07-16 11:38:34.681045084 +0100
+++ src/x86/ffi64.c     2012-07-16 22:34:42.959552750 +0100
@@ -38,7 +38,7 @@
 #define MAX_SSE_REGS 8

 #ifdef __INTEL_COMPILER
-#define UINT128 __m128
+typedef struct { int64_t m[2]; } __int128_t;
 #else
 #define UINT128 __int128_t
 #endif
@@ -47,7 +47,7 @@
 {
   /* Registers for argument passing.  */
   UINT64 gpr[MAX_GPR_REGS];
-  UINT128 sse[MAX_SSE_REGS];
+  __int128_t sse[MAX_SSE_REGS];
 };

 extern void ffi_call_unix64 (void *args, unsigned long bytes, unsigned flags,
@@ -477,10 +477,20 @@
                  break;
                case X86_64_SSE_CLASS:
                case X86_64_SSEDF_CLASS:
+#ifdef __INTEL_COMPILER
+                 reg_args->sse[ssecount].m[0] = *(UINT64 *) a;
+                 reg_args->sse[ssecount++].m[1] = 0;
+#else
                  reg_args->sse[ssecount++] = *(UINT64 *) a;
+#endif
                  break;
                case X86_64_SSESF_CLASS:
+#ifdef __INTEL_COMPILER
+                 reg_args->sse[ssecount].m[0] = *(UINT32 *) a;
+                 reg_args->sse[ssecount++].m[1] = 0;
+#else
                  reg_args->sse[ssecount++] = *(UINT32 *) a;
+#endif
                  break;
                default:
                  abort();
msg165661 - (view) Author: Alex Leach (Alex.Leach) Date: 2012-07-16 22:18
It skips 55, sorry, passing 1659.
msg165662 - (view) Author: Alex Leach (Alex.Leach) Date: 2012-07-16 22:25
That's the same patch as I attached before actually, so sorry for the spam..
msg192232 - (view) Author: Jens Timmerman (Jens.Timmerman) Date: 2013-07-03 11:58
I believe this pull request fixed this upstream (it did for me)
https://github.com/atgreen/libffi/pull/43

I fixed this by including "xmmintrin.h" instead of switching to 
reg_args->sse[ssecount].m (as per http://software.intel.com/en-us/forums/topic/303826 )
msg205685 - (view) Author: Jens Timmerman (Jens.Timmerman) Date: 2013-12-09 12:41
Since this is fixed in upstream libffi, can this be synced with the libffi included in python?
msg206152 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2013-12-13 22:06
Our included libffi is:

2013-03-17  Anthony Green  <green@moxielogic.com>

        * README: Update for 3.0.13.
        * configure.ac: Ditto.
        * configure: Rebuilt.
        * doc/*: Update version.


According to https://sourceware.org/libffi/ that is the latest
released version. Did I miss something?
msg206469 - (view) Author: Jens Timmerman (Jens.Timmerman) Date: 2013-12-17 16:04
sorry for my confusion, 

libffi's website stated

libffi-3.0.14 was released on TBD. 

I must have missed the TBD part.
History
Date User Action Args
2013-12-17 16:04:10Jens.Timmermansetmessages: + msg206469
2013-12-13 22:06:05skrahsetmessages: + msg206152
2013-12-09 12:41:14Jens.Timmermansetmessages: + msg205685
2013-07-03 11:58:44Jens.Timmermansetnosy: + Jens.Timmerman
messages: + msg192232
2012-07-16 22:25:50Alex.Leachsetmessages: + msg165662
2012-07-16 22:18:30Alex.Leachsetmessages: + msg165661
2012-07-16 22:16:04Alex.Leachsetmessages: + msg165660
2012-07-16 10:56:25Alex.Leachsetmessages: + msg165587
2012-07-16 09:53:36skrahsetmessages: + msg165580
2012-07-16 09:39:24skrahsetmessages: + msg165579
2012-07-08 23:36:30meador.ingesetassignee: theller ->
messages: + msg165045
stage: needs patch
2012-04-16 18:15:51Alex.Leachsetmessages: + msg158491
2012-04-16 13:14:02loewissetmessages: + msg158436
2012-04-15 22:47:30Alex.Leachsetmessages: + msg158378
2012-04-15 22:31:09Arfreversetmessages: + msg158376
2012-04-15 22:11:23meador.ingesetmessages: + msg158373
2012-04-15 18:59:20Arfreversetnosy: + Arfrever
messages: + msg158355
2012-04-15 16:02:10meador.ingesetnosy: + meador.inge
2012-04-15 12:27:52Alex.Leachsetfiles: + ffi64.c.patch
2012-04-15 12:14:27loewissetmessages: + msg158323
2012-04-13 19:13:13Alex.Leachsetfiles: + ffi64.c.patch

nosy: + Alex.Leach
messages: + msg158226

keywords: + patch
2010-07-22 21:21:03skrahsetnosy: + skrah
messages: + msg111219
2008-10-17 16:10:38loewissetmessages: + msg74923
2008-10-16 18:29:44jared.jenningssetmessages: + msg74842
2008-10-15 20:51:28loewissetnosy: + loewis
messages: + msg74814
2008-10-15 20:42:15jared.jenningscreate