x11/pixman: suggestions and a patch

[Alexey Dokuchaev]

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
-------------- next part --------------
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: