msg172381 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2012-10-08 14:33 |
I propose the addition of three new macros in pyport.h
#define PY_LITTLE_ENDIAN 1234
#define PY_BIG_ENDIAN 4321
#define PY_BYTE_ORDER <either little or big endian>
that are either set by a configure test or implemented like brg_endian.h. pyconfig.h has WORDS_BIGENDIAN which just checks for __BIG_ENDIAN__. The sysmodule has its own C code that checks the first byte of a long to detect the endianess.
http://hg.python.org/cpython/file/dd5e98ddcd39/Modules/_sha3/keccak/brg_endian.h
I'd like to remove Keccak's brg_endian.h (#16113 SHA-3 module) for something more official and useful to other code. I had to re-add the file because the tests were failing on a big endian Solaris (Sparc) machine.
|
msg172395 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2012-10-08 16:48 |
I agree, it is worth bring order to work with endianess. We can get rod of BYTEORDER_IS_BIG_ENDIAN and BYTEORDER_IS_LITTLE_ENDIAN in Objects/unicodeobject.c, IS_LITTLE_ENDIAN in Objects/longobject.c, tests in other modules. I don't see the necessity in new macros. WORDS_BIGENDIAN is enough.
If something failing on a big endian Solaris, build script should just define WORDS_BIGENDIAN.
|
msg172397 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2012-10-08 17:22 |
Here is a patch which removes some unnecessary defines and tests.
|
msg172399 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-10-08 18:30 |
This doesn't depend on #16113 (quite the contrary).
|
msg172400 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-10-08 18:32 |
> #define PY_LITTLE_ENDIAN 1234
> #define PY_BIG_ENDIAN 4321
> #define PY_BYTE_ORDER <either little or big endian>
Ouch, sounds confusing. I would rather have PY_LITTLE_ENDIAN defined only on little-endian machines and PY_BIG_ENDIAN only on big-endian machines.
(and PY_BYTE_ORDER isn't necessary)
|
msg172401 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2012-10-08 18:50 |
> Ouch, sounds confusing. I would rather have PY_LITTLE_ENDIAN defined only
> on little-endian machines and PY_BIG_ENDIAN only on big-endian machines.
> (and PY_BYTE_ORDER isn't necessary)
Why use two complementary boolean variables for a single boolean value (Python
does not support mixed endian in any case)? There is WORDS_BIGENDIAN already.
|
msg172403 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-10-08 19:02 |
> > Ouch, sounds confusing. I would rather have PY_LITTLE_ENDIAN defined only
> > on little-endian machines and PY_BIG_ENDIAN only on big-endian machines.
> > (and PY_BYTE_ORDER isn't necessary)
>
> Why use two complementary boolean variables for a single boolean value (Python
> does not support mixed endian in any case)?
Because it's simply more practical than having to remember which one
exists :-)
> There is WORDS_BIGENDIAN already.
A rather unfortunate IMHO.
|
msg172416 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2012-10-08 21:00 |
Just a reminder that we support configurations where there are both big-endian and little-endinan binaries in the *same* executable file: for example, Mac OS X 32-bit-only universal binaries (.so, .dylib, .exe) contain both i386 and ppc code in the same file. So any -endian differences must remain isolated to C code or be dynamically tested at execution time in Python code. And ./configure time tests can be problematic. (The proposed patch doesn't have that problem but it might be an issue later in tests.)
|
msg172427 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2012-10-09 00:45 |
My proposal mimics the API of endian.h. It defines a BYTE_ORDER macro that is either equal to LITTLE_ENDIAN, BIG_ENDIAN or PDB_ENDIAN (aka mixed endian). I need it that way for Keccak but I can roll my own definitions if you prefer just two macros.
If you prefer two macros instead of three then we should name them PY_IS_LITTLE_ENDIAN and PY_IS_BIG_ENDIAN.
Ned:
If I understand you correctly than we can't have a configure definition and need macro magic like brg_endian.h.
|
msg172431 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2012-10-09 01:38 |
Christian: That's right because there is only one configure execution in the OS X universal builds and only compiler call per module just like a normal single-architecture unix build. Under the covers, the Apple compiler driver transparently makes multiple compiler calls, one for each architecture, and then use lipo to combine the binaries into one .o file etc.
|
msg172469 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-10-09 10:17 |
> If you prefer two macros instead of three then we should name them
> PY_IS_LITTLE_ENDIAN and PY_IS_BIG_ENDIAN.
Fine with me. Having three macros is pointless complication.
> If I understand you correctly than we can't have a configure definition
> and need macro magic like brg_endian.h.
Replicating the current WORDS_BIGENDIAN logic should be enough.
|
msg172484 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2012-10-09 15:07 |
I think it's not enough as some platforms prefix/suffix the macros with dashes. The values must be compared to BYTE_ORDER iff the macro is defined. too. I've checked some machines of the Snakebite network and came up with this macro cascade:
/*
* The endianess (byte order) can be defined in several ways. Some platforms
* define either LITTLE_ENDIAN or BIG_ENDIAN while other platforms define
* both and require comparison to BYTE_ORDER. Sometimes the macros are
* prefixed or suffixed with one or two dashes, too. SPAM seems to be an
* alias of __SPAM an all platforms that have SPAM.
* The endian check can't be handled in a configure step as it would break
* Apple's universal builds.
*/
#if defined(__BYTE_ORDER)
# if defined(__BIG_ENDIAN) && __BYTE_ORDER == __BIG_ENDIAN
# define PY_IS_BIG_ENDIAN 1
# elif defined(__LITTLE_ENDIAN) && __BYTE_ORDER == __LITTLE_ENDIAN
# define PY_IS_LITTLE_ENDIAN 1
# else
# error "__BYTE_ORDER not in (__BIG_ENDIAN, __LITTLE_ENDIAN)"
# endif
#elif defined(__BIG_ENDIAN) && !defined(__LITTLE_ENDIAN)
# define PY_IS_BIG_ENDIAN 1
#elif defined(__LITTLE_ENDIAN) && !defined(__BIG_ENDIAN)
# define PY_IS_LITTLE_ENDIAN 1
/* and know the same block with just one dash as prefix */
#elif defined(_BYTE_ORDER)
# if defined(_BIG_ENDIAN) && _BYTE_ORDER == _BIG_ENDIAN
# define PY_IS_BIG_ENDIAN 1
# elif defined(_LITTLE_ENDIAN) && _BYTE_ORDER == _LITTLE_ENDIAN
# define PY_IS_LITTLE_ENDIAN 1
# else
# error "_BYTE_ORDER not in (_BIG_ENDIAN, _LITTLE_ENDIAN)"
# endif
#elif defined(_BIG_ENDIAN) && !defined(_LITTLE_ENDIAN)
# define PY_IS_BIG_ENDIAN 1
#elif defined(_LITTLE_ENDIAN) && !defined(_BIG_ENDIAN)
# define PY_IS_LITTLE_ENDIAN 1
/* extra apple */
#elif defined(__APPLE__)
# if defined(__BIG_ENDIAN__)
# define PY_IS_BIG_ENDIAN 1
# elif defined(__LITTLE_ENDIAN__)
# define PY_IS_LITTLE_ENDIAN 1
# else
# error "__APPLE__ but neither __BIG_ENDIAN__ nor __LITTLE_ENDIAN__"
# endif
/* Windows */
#elif defined(WIN32)
# define PY_IS_LITTLE_ENDIAN 1
#else
# error "Unable to detect endianess"
#endif
|
msg172491 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2012-10-09 16:32 |
> Fine with me. Having three macros is pointless complication.
Don't having two macros for one boolean value pointless complication too?
|
msg172499 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-10-09 18:37 |
> > Fine with me. Having three macros is pointless complication.
>
> Don't having two macros for one boolean value pointless complication too?
No, since you need only one to get going. Either:
#ifdef PY_LITTLE_ENDIAN
...
or:
#ifdef PY_BIG_ENDIAN
...
So this is really a simplification compared to the awkward:
#ifdef PY_ENDIAN == BIG_ENDIAN
|
msg172503 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2012-10-09 18:51 |
Usually you need code for both alternatives.
#ifdef WORDS_BIGENDIAN
...
#else
...
#endif
What can be simpler? Of cause, using *two* macros is complicated. You need
only one and second macros is unnecessary.
|
msg172522 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-10-09 20:42 |
AC_C_BIGENDIAN
|
msg172534 - (view) |
Author: Roumen Petrov (rpetrov) * |
Date: 2012-10-09 21:38 |
As Victor point configure script already check for endian (macro AC_C_BIGENDIAN) but you should ask python OS X guru to review .
Macro is not adjusted to python needs as action for universal build is not defined yet. The default is to define AC_APPLE_UNIVERSAL_BUILD .
Otherwise WORDS_BIGENDIAN is defined for bigendian .
|
msg172557 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2012-10-10 09:42 |
The patch defines either PY_IS_BIG_ENDIAN or PY_IS_LITTLE_ENDIAN and sets PY_LITTLE_ENDIAN_FLAG to either 0 or 1 for some modules that require a boolean flag.
|
msg173205 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-10-17 21:09 |
I would prefer if you yanked PY_LITTLE_ENDIAN_FLAG. We don't need two different ways to do it.
|
msg173210 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2012-10-17 21:46 |
I've replaced the macros with PY_BIG_ENDIAN or PY_LITTLE_ENDIAN. Both are always defined and exactly one is set to 1, the other to 0.
|
msg173211 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-10-17 21:48 |
Looks fine to me. I suppose you'll have to check the big endian buildbots.
|
msg173213 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2012-10-17 21:52 |
New changeset d2127cdec10e by Christian Heimes in branch 'default':
Issue #16166: Add PY_LITTLE_ENDIAN and PY_BIG_ENDIAN macros and unified
http://hg.python.org/cpython/rev/d2127cdec10e
|
msg173235 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2012-10-18 07:35 |
> I've replaced the macros with PY_BIG_ENDIAN or PY_LITTLE_ENDIAN. Both are
> always defined and exactly one is set to 1, the other to 0.
It looks like a joke.
The programmer puts on the table by his bed two glasses. One with water, for
the case of he will drink at night. Another empty, for the case of he will not
drink.
|
msg173348 - (view) |
Author: Jesús Cea Avión (jcea) * |
Date: 2012-10-19 17:27 |
Instead of using macro values, I would rather define ONLY a single macro, PY_LITTLE_ENDIAN or PY_BIG_ENDIAN.
Far less error prone!.
It is far fool-proof "#ifdef PY_LITTLE_ENDIAN" than "#if PY_LITTLE_ENDIAN".
|
msg173349 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2012-10-19 17:43 |
> Instead of using macro values, I would rather define ONLY a single macro,
> PY_LITTLE_ENDIAN or PY_BIG_ENDIAN.
Time machine strikes back.
#ifdef WORDS_BIGENDIAN
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:37 | admin | set | github: 60370 |
2012-10-19 17:43:13 | serhiy.storchaka | set | messages:
+ msg173349 |
2012-10-19 17:27:40 | jcea | set | messages:
+ msg173348 |
2012-10-18 07:35:59 | serhiy.storchaka | set | messages:
+ msg173235 |
2012-10-17 22:25:55 | christian.heimes | set | status: open -> closed resolution: fixed stage: commit review -> resolved |
2012-10-17 21:52:29 | python-dev | set | nosy:
+ python-dev messages:
+ msg173213
|
2012-10-17 21:48:15 | pitrou | set | messages:
+ msg173211 stage: needs patch -> commit review |
2012-10-17 21:46:20 | christian.heimes | set | files:
+ endian2.patch
messages:
+ msg173210 |
2012-10-17 21:09:36 | pitrou | set | messages:
+ msg173205 |
2012-10-10 09:42:51 | christian.heimes | set | files:
+ endian.patch
messages:
+ msg172557 |
2012-10-10 00:24:47 | jcea | set | nosy:
+ jcea
|
2012-10-09 21:38:44 | rpetrov | set | nosy:
+ rpetrov messages:
+ msg172534
|
2012-10-09 20:42:49 | vstinner | set | nosy:
+ vstinner messages:
+ msg172522
|
2012-10-09 18:51:07 | serhiy.storchaka | set | messages:
+ msg172503 |
2012-10-09 18:37:45 | pitrou | set | messages:
+ msg172499 |
2012-10-09 16:32:25 | serhiy.storchaka | set | messages:
+ msg172491 |
2012-10-09 15:30:14 | Arfrever | set | nosy:
+ Arfrever
|
2012-10-09 15:07:42 | christian.heimes | set | messages:
+ msg172484 |
2012-10-09 10:17:10 | pitrou | set | messages:
+ msg172469 |
2012-10-09 01:38:59 | ned.deily | set | messages:
+ msg172431 |
2012-10-09 00:45:49 | christian.heimes | set | messages:
+ msg172427 |
2012-10-08 21:00:19 | ned.deily | set | nosy:
+ ned.deily messages:
+ msg172416
|
2012-10-08 19:02:21 | pitrou | set | messages:
+ msg172403 |
2012-10-08 18:50:16 | serhiy.storchaka | set | messages:
+ msg172401 |
2012-10-08 18:32:11 | pitrou | set | messages:
+ msg172400 |
2012-10-08 18:30:46 | pitrou | set | dependencies:
- Add SHA-3 and SHAKE (Keccak) support messages:
+ msg172399 |
2012-10-08 17:22:52 | serhiy.storchaka | set | files:
+ endianess_cleanup.patch keywords:
+ patch messages:
+ msg172397
|
2012-10-08 16:48:31 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg172395
|
2012-10-08 14:33:55 | christian.heimes | set | nosy:
+ pitrou dependencies:
+ Add SHA-3 and SHAKE (Keccak) support
|
2012-10-08 14:33:27 | christian.heimes | create | |