Date: Fri, 20 Jan 2012 20:33:11 +0700 From: Alexey Dokuchaev <danfe@nsu.ru> To: x11@freebsd.org Subject: x11/pixman: suggestions and a patch Message-ID: <20120120133310.GA83690@regency.nsu.ru>
next in thread | raw e-mail | index | archive | help
--5vNYLRcllDrimb99
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Hi there,
I was always confused when rebuilding x11/pixman by these lines of Makefile:
.if defined(WITHOUT_SIMD)
CONFIGURE_ARGS= --disable-vmx --disable-arm-simd
.if ${ARCH:Namd64}
CONFIGURE_ARGS+= --disable-mmx --disable-sse2
.endif
.endif
1) ${ARCH:Namd64} is hardly readable, we usually spell it as ${ARCH} != "amd64"
2) superfluous blank lines make this code hard to read as well
3) also, it seems that features handling is not complete for current version
of pixman, per ./configure --help:
--disable-openmp do not use OpenMP
--disable-mmx disable x86 MMX fast paths
--disable-sse2 disable SSE2 fast paths
--disable-vmx disable VMX fast paths
--disable-arm-simd disable ARM SIMD fast paths
--disable-arm-neon disable ARM NEON fast paths
--disable-arm-iwmmxt disable ARM IWMMXT fast paths
Most importantly, we usually check for CPU capabilities by examining
MACHINE_CPU variable. While it currently does not have proper support for
ARM (but then again, neither does our ports collection), MMX and SSE2
detection is normally done this way (cf. security/john, www/chromium).
As a bonus, check for amd64 (where MMX and SSE2 are always present) is no
longer required. Default package on cluster would not change, as it is
built for default CPUTYPE (i486), which ensures conservative options.
Do you think we should flip the SIMD knob to on by default maybe?
4) you might consider adding --disable-arm-neon and --disable-arm-iwmmxt,
and maybe prodide an OPTION for OpenMP support (but I won't touch these
things since I lack proper hardware support to test)
5) I am also not sure why post-patch: sed(1) hack for configure script is
needed: isn't --disable-gtk not enough?
Please consider (approve or commit yourself) attach patch. I've rephrased
OPTION line a bit to fit in 80-char terminal while I am here.
./danfe
--5vNYLRcllDrimb99
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="pixman.diff"
Index: Makefile
===================================================================
RCS file: /home/danfe/fbsd/FreeBSD-CVS/ports/x11/pixman/Makefile,v
retrieving revision 1.20
diff -u -r1.20 Makefile
--- Makefile 28 Nov 2011 23:35:08 -0000 1.20
+++ Makefile 20 Jan 2012 13:23:54 -0000
@@ -17,17 +17,18 @@
USE_PERL5_BUILD=yes
USE_GNOME= ltverhack:9
-OPTIONS= SIMD "Enable autodetection of SIMD features (MMX, SSE2, VMX)" off
+OPTIONS= SIMD "Enable SIMD features autodetection (MMX, SSE2, VMX)" off
.include <bsd.port.pre.mk>
.if defined(WITHOUT_SIMD)
CONFIGURE_ARGS= --disable-vmx --disable-arm-simd
-
-.if ${ARCH:Namd64}
-CONFIGURE_ARGS+= --disable-mmx --disable-sse2
-.endif
-
+. if ! ${MACHINE_CPU:Mmmx}
+CONFIGURE_ARGS+= --disable-mmx
+. endif
+. if ! ${MACHINE_CPU:Msse2}
+CONFIGURE_ARGS+= --disable-sse2
+. endif
.endif
post-patch:
--5vNYLRcllDrimb99--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120120133310.GA83690>