Re: Including limits.h in gmp-impl.h
On 2014-01-03 12:19:26 +0100, Marc Glisse wrote: On Fri, 3 Jan 2014, Torbjorn Granlund wrote: Vincent Lefevre vinc...@vinc17.net writes: Note that if you want to care of non-two's-complement implementations, you should write: Isn't the notion of 2's complement restricted to signed types? Yes, but in ~0, 0 is of signed type. In non-two's-complement implementations, 0 may have several representations, so that ~0 is not guaranteed to work. #define __GMP_USHRT_MAX (0 + (unsigned short) -1) The previous form has worked so far, and I'd like to remove the macro completely soon, so it doesn't seem worth it. Is that out-of-range conversion really Proper C? unsigned = modular arithmetic, so yes, it is fine. Anyway if -1 didn't work, then ~0 wouldn't have worked either, because the result of ~0 is -1. Not to be confused with ~0U or ~ (unsigned int) 0. -- Vincent Lefèvre vinc...@vinc17.net - Web: http://www.vinc17.net/ 100% accessible validated (X)HTML - Blog: http://www.vinc17.net/blog/ Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon) ___ gmp-devel mailing list gmp-devel@gmplib.org https://gmplib.org/mailman/listinfo/gmp-devel
Re: Including limits.h in gmp-impl.h
On Thu, 2 Jan 2014, Niels Möller wrote: Marc Glisse marc.gli...@inria.fr writes: I'll also push a patch handling the fact that the standard SHRT_MAX and others don't have type short but int. Can you explain what the problem is? I noticed this change, --- a/gmp-h.in Thu Jan 02 12:28:21 2014 +0100 +++ b/gmp-h.in Thu Jan 02 13:25:31 2014 +0100 @@ -437,10 +437,10 @@ #define __GMP_MAX(h,i) ((h) (i) ? (h) : (i)) /* __GMP_USHRT_MAX is not ~ (unsigned short) 0 because short is promoted - to int by ~. */ + to int by ~. It still needs to have the promoted type. */ #define __GMP_UINT_MAX (~ (unsigned) 0) #define __GMP_ULONG_MAX (~ (unsigned long) 0) -#define __GMP_USHRT_MAX ((unsigned short) ~0) +#define __GMP_USHRT_MAX (0 + (unsigned short) ~0) Questions: 1. Why should it be of type int? Which problems are caused by having it as unsigned short? 2. If int is desired, I think it's a bit clearer with an explicit cast, #define __GMP_USHRT_MAX ((int) (unsigned short) ~0) But, unconditionally defining it as int also seems unportable. A C implementation may define both short and int to be 32 bits, right? Then the maximum unsigned short doesn't fit in a signed int. While it would always fit in an *unsigned* int. Is that what the 0 + promotion thing is intended to do; you know the spec a better than me, but you'd get unsigned int if short and int are of the same size, and signed int if it's of larger size than short? I miss a comment spelling out which type really is intended, and why. You have pretty much said everything already ;-) C99 (it's the only version I have here) says about SHRT_MAX and others: the following shall be replaced by expressions that have the same type as would an expression that is an object of the corresponding type converted according to the integer promotions. integer promotion for a short means int, and for an unsigned short it means either int or unsigned int depending on their relative size. 0 + is indeed a random way to force type promotion (note the sentence I added in the comment above __GMP_USHRT_MAX). I made the change so that the type of USHRT_MAX is the same whether it comes from limits.h or from gmp. The type is mostly irrelevant, except that we have a few printf calls in the testsuite where we were using %hX. For gmp-5.3 (not this one, the one after that), I think we should remove __GMP_USHRT_MAX altogether and forcibly include limits.h in gmp.h (it isn't comparable to stdio.h). We would also remove all the related code in gmp-impl.h, and half of my patch would become irrelevant. (I didn't want to do it so close to the release, but it would only take a few minutes if you want me to do it for 5.2) So, do you agree with the change? (care to add a suitable comment?) Or would you prefer a different change? -- Marc Glisse ___ gmp-devel mailing list gmp-devel@gmplib.org https://gmplib.org/mailman/listinfo/gmp-devel
Re: Including limits.h in gmp-impl.h
Marc Glisse marc.gli...@inria.fr writes: So, do you agree with the change? Yes. I don't understand why the C spec doesn't define USHORT_MAX the way it does, but we should nevertheless define our constant in the same way as C's USHORT_MAX. (care to add a suitable comment?) I think the comment could be improved. But if you intend to remove the definition soon anyway, it doesn't matter. Regards, /Niels -- Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26. Internet email is subject to wholesale government surveillance. ___ gmp-devel mailing list gmp-devel@gmplib.org https://gmplib.org/mailman/listinfo/gmp-devel
Re: Including limits.h in gmp-impl.h
On Fri, 27 Dec 2013, Torbjorn Granlund wrote: Marc Glisse marc.gli...@inria.fr writes: I couldn't find anything other than the -mcpu=ultrasparc ABI=32 issue with gcc-2.9x around 1999-2000. I haven't looked deeply into this issue, but to me it seems strange that we define INT_MAX etc ourselves. I think we might trust limits.h these days. If a 15 year old system then gets desupported, too bad. Then I am considering pushing the attached patch soon. -- Marc Glissediff -r 709a37323927 ChangeLog --- a/ChangeLog Fri Dec 27 07:50:29 2013 +0100 +++ b/ChangeLog Fri Dec 27 15:02:47 2013 +0100 @@ -1,3 +1,9 @@ +2013-12-27 Marc Glisse marc.gli...@inria.fr + + * configure.ac: Test for limits.h. + * gmp-impl.h: Include limits.h if available. + * tests/mpn/t-get_d.c: Remove comment about limits.h + 2013-12-26 Marco Bodrato bodr...@mail.dm.unipi.it * Update many file's encoding to UTF-8. diff -r 709a37323927 configure.ac --- a/configure.ac Fri Dec 27 07:50:29 2013 +0100 +++ b/configure.ac Fri Dec 27 15:02:47 2013 +0100 @@ -2597,7 +2597,7 @@ # inttypes.h, stdint.h, unistd.h and sys/types.h are already in the autoconf # default tests # -AC_CHECK_HEADERS(fcntl.h float.h invent.h langinfo.h locale.h nl_types.h sys/attributes.h sys/iograph.h sys/mman.h sys/param.h sys/processor.h sys/pstat.h sys/sysinfo.h sys/syssgi.h sys/systemcfg.h sys/time.h sys/times.h) +AC_CHECK_HEADERS(fcntl.h float.h invent.h langinfo.h limits.h locale.h nl_types.h sys/attributes.h sys/iograph.h sys/mman.h sys/param.h sys/processor.h sys/pstat.h sys/sysinfo.h sys/syssgi.h sys/systemcfg.h sys/time.h sys/times.h) # On SunOS, sys/resource.h needs sys/time.h (for struct timeval) AC_CHECK_HEADERS(sys/resource.h,,, diff -r 709a37323927 gmp-impl.h --- a/gmp-impl.hFri Dec 27 07:50:29 2013 +0100 +++ b/gmp-impl.hFri Dec 27 15:02:47 2013 +0100 @@ -36,9 +36,9 @@ #include intrinsics.h /* for _popcnt */ #endif -/* limits.h is not used in general, since it's an ANSI-ism, and since on - solaris gcc 2.95 under -mcpu=ultrasparc in ABI=32 ends up getting wrong - values (the ABI=64 values). +/* For INT_MAX, etc. We used to avoid it because of a bug (on solaris, + gcc 2.95 under -mcpu=ultrasparc in ABI=32 ends up getting wrong + values (the ABI=64 values)), but it should be safe now. On Cray vector systems, however, we need the system limits.h since sizes of signed and unsigned types can differ there, depending on compiler @@ -46,7 +46,7 @@ reference, int can be 46 or 64 bits, whereas uint is always 64 bits; and short can be 24, 32, 46 or 64 bits, and different for ushort. */ -#if defined _CRAY +#if HAVE_LIMITS_H #include limits.h #endif diff -r 709a37323927 tests/mpn/t-get_d.c --- a/tests/mpn/t-get_d.c Fri Dec 27 07:50:29 2013 +0100 +++ b/tests/mpn/t-get_d.c Fri Dec 27 15:02:47 2013 +0100 @@ -17,12 +17,6 @@ You should have received a copy of the GNU General Public License along with the GNU MP Library test suite. If not, see https://www.gnu.org/licenses/. */ -/* Note that we don't use limits.h for LONG_MIN, but instead our own - definition in gmp-impl.h. In gcc 2.95.4 (debian 3.0) under - -mcpu=ultrasparc, limits.h sees __sparc_v9__ defined and assumes that - means long is 64-bit long, but it's only 32-bits, causing fatal compile - errors. */ - #include config.h #include setjmp.h ___ gmp-devel mailing list gmp-devel@gmplib.org https://gmplib.org/mailman/listinfo/gmp-devel
Re: Including limits.h in gmp-impl.h
Marc Glisse marc.gli...@inria.fr writes: Then I am considering pushing the attached patch soon. Do we really need a configure test? Which C compilers lack limits.h? Regards, /Niels -- Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26. Internet email is subject to wholesale government surveillance. ___ gmp-devel mailing list gmp-devel@gmplib.org https://gmplib.org/mailman/listinfo/gmp-devel
Re: Including limits.h in gmp-impl.h
On Fri, 27 Dec 2013, Niels Möller wrote: Marc Glisse marc.gli...@inria.fr writes: Then I am considering pushing the attached patch soon. Do we really need a configure test? No idea. It seemed safer, and since we already have fallback code... On the other hand, mini-gmp seems happy enough with limits.h. If we really want to assume limits.h exists and works, there is more cleanup that can be done in gmp-impl.h (remove the fallback code for INT_MAX etc). -- Marc Glisse ___ gmp-devel mailing list gmp-devel@gmplib.org https://gmplib.org/mailman/listinfo/gmp-devel
Including limits.h in gmp-impl.h
Hello, trying to compile gmp with clang, I got this error in the C++ testsuite: In file included from /usr/include/limits.h:152: /usr/include/x86_64-linux-gnu/bits/xopen_lim.h:97:7: error: token is not a valid binary operator in a preprocessor subexpression # if INT_MAX == 2147483647 ^~~ /data/repos/gmp/gmp-impl.h:579:31: note: expanded from macro 'INT_MAX' #define INT_MAX(-(INT_MIN+1)) ^~~ /data/repos/gmp/gmp-impl.h:576:35: note: expanded from macro 'INT_MIN' #define INT_MIN((int) UINT_HIGHBIT) ~ ^ /data/repos/gmp/gmp-impl.h:564:28: note: expanded from macro 'UINT_HIGHBIT' #define UINT_HIGHBIT (UINT_MAX ^ ((unsigned) UINT_MAX 1)) ^ Something in the glibc headers doesn't like the gmp definitions of those macros. It is easily worked around by including limits.h in gmp-impl.h, but I see this comment: /* limits.h is not used in general, since it's an ANSI-ism, and since on solaris gcc 2.95 under -mcpu=ultrasparc in ABI=32 ends up getting wrong values (the ABI=64 values). On Cray vector systems, however, we need the system limits.h since sizes of signed and unsigned types can differ there, depending on compiler options (eg. -hnofastmd), making our SHRT_MAX etc expressions fail. For reference, int can be 46 or 64 bits, whereas uint is always 64 bits; and short can be 24, 32, 46 or 64 bits, and different for ushort. */ #if defined _CRAY #include limits.h #endif So even using autoconf to check if limits.h exists may not be enough. Or can we ignore the old solaris issue? -- Marc Glisse ___ gmp-devel mailing list gmp-devel@gmplib.org https://gmplib.org/mailman/listinfo/gmp-devel
Re: Including limits.h in gmp-impl.h
Oups, looks like I already asked about that: https://gmplib.org/list-archives/gmp-bugs/2011-November/002443.html and the reply was to try including tests.h before gmp-impl.h. -- Marc Glisse ___ gmp-devel mailing list gmp-devel@gmplib.org https://gmplib.org/mailman/listinfo/gmp-devel
Re: Including limits.h in gmp-impl.h
Vincent Lefevre vinc...@vinc17.net writes: On 2013-12-25 12:13:39 +0100, Marc Glisse wrote: Oups, looks like I already asked about that: https://gmplib.org/list-archives/gmp-bugs/2011-November/002443.html and the reply was to try including tests.h before gmp-impl.h. I'd say that it would be better to fix GMP. Reusing standard macros (even when there are not supposed to be defined under some conditions) is ugly. For instance, GMP_INT_MAX could be used instead of INT_MAX, and there would be no risk of clashes. Including tests.h before gmp-impl.h is probably not a good solution because in such tests, limits.h (from tests.h) may be used instead of GMP's definitions. So, if don't trust limits.h, that's incorrect. I'd also say that if limits.h is incorrect, lots of programs will fail to build correctly. So, GMP's trick to redefine limits will be useful only for users who use GMP in their own applications, without using limits.h in their applications either. IIRC, limits.h was unreliable for many years, and our workarounds allowed GMP to work on more systems. -- Torbjörn ___ gmp-devel mailing list gmp-devel@gmplib.org https://gmplib.org/mailman/listinfo/gmp-devel
Re: Including limits.h in gmp-impl.h
On 2013-12-25 13:28:20 +0100, Torbjorn Granlund wrote: IIRC, limits.h was unreliable for many years, and our workarounds allowed GMP to work on more systems. I've tried to find something about that on Google, but couldn't find anything. Any reference? -- Vincent Lefèvre vinc...@vinc17.net - Web: http://www.vinc17.net/ 100% accessible validated (X)HTML - Blog: http://www.vinc17.net/blog/ Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon) ___ gmp-devel mailing list gmp-devel@gmplib.org https://gmplib.org/mailman/listinfo/gmp-devel
Re: Including limits.h in gmp-impl.h
Vincent Lefevre vinc...@vinc17.net writes: I've tried to find something about that on Google, but couldn't find anything. Any reference? Perhaps ChangeLog has some history. Torbjörn ___ gmp-devel mailing list gmp-devel@gmplib.org https://gmplib.org/mailman/listinfo/gmp-devel
