Date: Tue, 31 Jan 2017 16:26:55 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: "Conrad E. Meyer" <cem@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r313006 - in head: sys/conf sys/libkern sys/libkern/x86 sys/sys tests/sys/kern Message-ID: <20170131153411.G1061@besplex.bde.org> In-Reply-To: <201701310326.v0V3QW30024375@repo.freebsd.org> References: <201701310326.v0V3QW30024375@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 31 Jan 2017, Conrad E. Meyer wrote:
> Log:
> calculate_crc32c: Add SSE4.2 implementation on x86
This breaks building with gcc-4.2.1, and depends on using non-kernel clang
headers for clang.
> Modified: head/sys/conf/files.amd64
> ==============================================================================
> --- head/sys/conf/files.amd64 Tue Jan 31 01:55:29 2017 (r313005)
> +++ head/sys/conf/files.amd64 Tue Jan 31 03:26:32 2017 (r313006)
> @@ -593,6 +593,11 @@ compat/ndis/subr_pe.c optional ndisapi
> compat/ndis/subr_usbd.c optional ndisapi pci
> compat/ndis/winx64_wrap.S optional ndisapi pci
> #
> +crc32_sse42.o standard \
I don't want it, but it is standard.
> + dependency "$S/libkern/x86/crc32_sse42.c" \
> + compile-with "${CC} -c ${CFLAGS:N-nostdinc} ${WERROR} ${PROF} -msse4 ${.IMPSRC}" \
-msse4 is not supported by gcc-4.2.1,
Removing nostdinc pollutes the build with host headers, and the one needed
might not be installed, and it doesn't exist for gcc-4.2.1.
Similarly for i386.
> Modified: head/sys/libkern/crc32.c
> ==============================================================================
> --- head/sys/libkern/crc32.c Tue Jan 31 01:55:29 2017 (r313005)
> +++ head/sys/libkern/crc32.c Tue Jan 31 03:26:32 2017 (r313006)
> @@ -46,8 +46,14 @@
> __FBSDID("$FreeBSD$");
>
> #include <sys/param.h>
> +#include <sys/libkern.h>
Style bug. libkern.h is part if systm.h.
> #include <sys/systm.h>
Ordering bug. systm.h is a prerequisite for all kernel headers except
param.h, since it defines macros which might be used by other headers.
> Added: head/sys/libkern/x86/crc32_sse42.c
> ==============================================================================
> --- /dev/null 00:00:00 1970 (empty, because file is newly added)
> +++ head/sys/libkern/x86/crc32_sse42.c Tue Jan 31 03:26:32 2017 (r313006)
> @@ -0,0 +1,288 @@
> ...
> +#ifdef USERSPACE_TESTING
> +#include <stdint.h>
> +#else
> +#include <sys/param.h>
> +#include <sys/kernel.h>
> +#include <sys/libkern.h>
> +#include <sys/systm.h>
> +#endif
Style and ordering bugs, as above.
> +
> +#include <nmmintrin.h>
This header is outside of the kernel source tree. It is not even in
/usr/include, but clang finds it in:
crc32_sse42.o:
/usr/bin/../lib/clang/3.9.0/include/nmmintrin.h \
/usr/bin/../lib/clang/3.9.0/include/smmintrin.h \
/usr/bin/../lib/clang/3.9.0/include/tmmintrin.h \
/usr/bin/../lib/clang/3.9.0/include/pmmintrin.h \
/usr/bin/../lib/clang/3.9.0/include/emmintrin.h \
/usr/bin/../lib/clang/3.9.0/include/xmmintrin.h \
/usr/bin/../lib/clang/3.9.0/include/mmintrin.h \
/usr/bin/../lib/clang/3.9.0/include/f16cintrin.h \
/usr/bin/../lib/clang/3.9.0/include/popcntintrin.h
nmmintrin.h doesn't exist for gcc-4.2.1. gcc-4.2.1 has some of the other
intrin.h files, but they aren't installed in FreeBSD-9, and of course they
don't support newer SSE.
Inline asm is much less unportable than intrinsics. kib used the correct
method of .byte's in asms to avoid depending on assembler support for newer
instructions. .byte is still used for clflush on amd64 and i386. It
used to be used for invpcid on amd64. I can't find where it is or was
used for xsave stuff.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170131153411.G1061>