Re: Portability: common-src/amcrc32chw.c & config/amanda/amanda_configure.m4

Jean-Louis Martineau writes:
- What is the issue you get?

NetBSD does not provide __builtin_ia32_* in any library.  In
experimentation, I did find that if -msse4.2 is provided to gcc,
the various __builtin_ia32_ functions do appear.

However, pkgsrc attempts to build for lowest common denominiator
systems when building for distribution (hence, not using SSE* in
any form on x86 systems.)

Individual users can add additional build flags for their specific
systems, but the distribution builds must be portable for all
systems of a given processor/OS architecture.

- I think I can replace the #ifdef line with:
- #ifdef __SSE_4_2_
- 
- Because if __SSE_4_2_ is defined then the __builtin_ia32_crc32* function
- must be defined

Depending on a compiler specific internal macro definition seems
fraught with danger.  Imagine the case of xycc (random non-gcc
compiler) defining __SSE_4_2_ for some purpose, but not
providing these particular functions.  Feature testing for the
functions used is almost always a better idea.

Do you have access to a non-Linux, non-x86, non-gcc system for
development/testing? (not all the world is linux/x86/gcc. :D )

Reflecting over lunch, perhaps it would be better to depend upon
OpenSSL/GnuTLS/LibreSSL to provide the CRC functions desired.
Those libraries would provide the appropriate machine specific
optimized versions for every platform, leaving Amanda more
system/compiler agnostic and portable.

- On 02/01/17 02:53 PM, Eric Schnoebelen wrote:
- > Issue: Attempting to use gcc __builtin functions without
- > appropriate feature tests.  Not all platforms use gcc, and not
- > all gcc platforms provide the __builtin_ia32_* functions (as not
- > all gcc platforms are necessarily x86_*)
- >
- > Solution: add feature test macros for the __builtin_ia32_*
- > functions being used, and wrapper their use with the feature
- > test macro.
- >
- > patches inline and as attachments
- >
- >
- > $NetBSD$
- >
- > Add tests for the gcc __builtin_ia32_crc32[qdsu]i functions.
- >
- > --- config/amanda/amanda_configure.m4.orig=092016-11-23 16:11:15.000000000 +0000
- > +++ config/amanda/amanda_configure.m4
- > @@ -364,8 +364,9 @@ ICE_CHECK_DECL(strcasecmp,string.h strin
- >   ICE_CHECK_DECL(euidaccess,unistd.h)
- >   ICE_CHECK_DECL(eaccess,unistd.h)
- >   ICE_CHECK_DECL(clock_gettime,time.h)
- > -AC_CHECK_FUNCS(getservbyname_r)
- > +ICE_CHECK_DECL(getservbyname_r, netdb.h)
- >   AC_CHECK_FUNCS(sem_timedwait)
- > +AC_CHECK_FUNCS(__builtin_ia32_crc32qi __builtin_ia32_crc32di __builtin_ia32_crc32si __builtin_ia32_crc32hi)
- >  
- >   #
- >   # Devices
- >
- >
- > $NetBSD$
- >
- > Change the test to use a feature test macro for the existance of the
- > builtin functions being used.
- >
- > (bad programmer, using compiler internal routines in public code.)
- >
- > --- common-src/amcrc32chw.c.orig=092016-11-23 16:11:16.000000000 +0000
- > +++ common-src/amcrc32chw.c
- > @@ -29,7 +29,11 @@
- >   #include <amutil.h>
- >   #include <amcrc32chw.h>
- >  
- > -#if defined __GNUC__ && GCC_VERSION > 40300 && (defined __x86_64__ || defined __i386__ || defined __i486__ || defined __i586__ || defined __i686__)
- > +#if defined HAVE___BUILTIN_IA32_CRC32QI && \
- > +	defined HAVE___BUILTIN_IA32_CRC32DI && \
- > +	defined HAVE___BUILTIN_IA32_CRC32SI && \
- > +	defined HAVE___BUILTIN_IA32_CRC32UI
- > +
- >   #define POLY 0x82F63B78
- >  
- >   /* Multiply a matrix times a vector over the Galois field of two elements,