Skip to contentSkip to conversation

package/lame: Fix compile on 32bit Intel with gcc-4.9.x

PATCH New Bernd Kuhls 3 participants 5 messages Aug 16, 2014 – Aug 28, 2014

lore
Share & tools
lore

Bernd Kuhls

author
#0 lore raw

PATCH 1/1 package/lame: Fix compile on 32bit Intel with gcc-4.9.x

In file included from xmm_quantize_sub.c:37:0:
/home/br/br/output/host/usr/lib/gcc/i586-buildroot-linux-uclibc/4.9.1/include/xmmintrin.h:205:1: error:
inlining failed in call to always_inline '_mm_sqrt_ps': target specific option mismatch _mm_sqrt_ps (__m128 __A)

To reproduce use this defconfig:

BR2_GCC_VERSION_4_9_X=y
BR2_PACKAGE_LAME=y

Signed-off-by Bernd Kuhls <[email protected]>
---
 package/lame/lame.mk |   10 ++++++++++
 1 file changed, 10 insertions(+)
package/lame/lame.mk +10 -0
index 938b759..eac4367 100644
@@ -23,6 +23,16 @@ ifeq ($(BR2_PACKAGE_NCURSES),y)
 LAME_DEPENDENCIES += ncurses
 endif
 
+# cross compiling on 32bit Intel is broken with gcc-4.9.x, for details see
+# http://lists.linuxfromscratch.org/pipermail/blfs-dev/2014-April/027259.html
+# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56145
+ifeq ($(BR2_i386)$(BR2_GCC_VERSION_4_9_X),yy)
+define LAME_COMPILE_FIX
+	$(SED) '/xmmintrin\.h/d' $(@D)/configure
+endef
+LAME_PRE_CONFIGURE_HOOKS += LAME_COMPILE_FIX
+endif
+
 ifeq ($(BR2_ENDIAN),"BIG")
 define LAME_BIGENDIAN_ARCH
 	echo "#define WORDS_BIGENDIAN 1" >>$(@D)/config.h
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH 1/1 package/lame: Fix compile on 32bit Intel with gcc-4.9.x

Dear Bernd Kuhls,

package/lame/lame.mk ifeq ($(BR2_PACKAGE_NCURSES),y) · line 34

Quoted 8 lines · Bernd Kuhls
package/lame/lame.mk +8 -0
+# http://lists.linuxfromscratch.org/pipermail/blfs-dev/2014-April/027259.html
+# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56145
+ifeq ($(BR2_i386)$(BR2_GCC_VERSION_4_9_X),yy)
+define LAME_COMPILE_FIX
+	$(SED) '/xmmintrin\.h/d' $(@D)/configure
+endef
+LAME_PRE_CONFIGURE_HOOKS += LAME_COMPILE_FIX
+endif

Unfortunately, this sort of check is not really acceptable, because it doesn't work with external toolchains: BR2_GCC_VERSION_* symbols are defined only for internal toolchains.

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#2 lore raw

PATCH 1/1 package/lame: Fix compile on 32bit Intel with gcc-4.9.x

Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8 @public.gmane.org> wrote in news:20140817093621.664fad88 at free- electrons.com:

Quoted 5 lines

Dear Bernd Kuhls,

Quoted 1 line · Bernd Kuhls

+# http://lists.linuxfromscratch.org/pipermail/blfs-dev/2014-

April/027259.html

Quoted 11 lines

Review

Quoted 7 lines
(quoted diff) +7 -0
+# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56145
+ifeq ($(BR2_i386)$(BR2_GCC_VERSION_4_9_X),yy)
+define LAME_COMPILE_FIX
+     $(SED) '/xmmintrin\.h/d' $(@D)/configure
+endef
+LAME_PRE_CONFIGURE_HOOKS += LAME_COMPILE_FIX
+endif

Unfortunately, this sort of check is not really acceptable, because it doesn't work with external toolchains: BR2_GCC_VERSION_* symbols are defined only for internal toolchains.

Hi Thomas,

I understand, but removing the gcc version check, which would remove support for xmmintrin.h in all i386 builds, would remove the mmx support for lame on many builds, where it is not necessary.

Would something like this, extended to all external toolchains, be acceptable?

toolchain/toolchain-external/Config.in +1 -0
@@ -18,6 +18,7 @@ config BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM
        select BR2_INSTALL_LIBSTDCPP
        select BR2_HOSTARCH_NEEDS_IA32_LIBS
        select BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_1
+       select BR2_GCC_VERSION_4_9_X
        help
          Linaro toolchain for the ARM architecture. It uses Linaro
          GCC 2014.05 (based on gcc 4.9), Linaro GDB 2014.05 (based on

In addition I could add a warning to package/lame/Config.in when gcc-4.9 is activated to inform users that mmx support is broken.

Regards, Bernd

#3 lore raw

PATCH 1/1 package/lame: Fix compile on 32bit Intel with gcc-4.9.x

Dear Bernd Kuhls,

Quoted 3 lines · Bernd Kuhls

I understand, but removing the gcc version check, which would remove support for xmmintrin.h in all i386 builds, would remove the mmx support for lame on many builds, where it is not necessary.

Yes, understood. Maybe instead add some conditional code in lame to enable the MMX stuff only with known working gcc versions? I think there are some #define you can test to get the gcc version.

toolchain/toolchain-external/Config.in config BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM · line 24

Quoted 13 lines

Would something like this, extended to all external toolchains, be acceptable?

toolchain/toolchain-external/Config.in +1 -0
@@ -18,6 +18,7 @@ config BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM
        select BR2_INSTALL_LIBSTDCPP
        select BR2_HOSTARCH_NEEDS_IA32_LIBS
        select BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_1
+       select BR2_GCC_VERSION_4_9_X
        help
          Linaro toolchain for the ARM architecture. It uses Linaro
          GCC 2014.05 (based on gcc 4.9), Linaro GDB 2014.05 (based on

That works only for "known" external toolchains: it won't work with custom external toolchains.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

PATCH 1/1 package/lame: Fix compile on 32bit Intel with gcc-4.9.x

Quoted 11 lines · Thomas Petazzoni

Dear Bernd Kuhls,

Quoted 3 lines · Bernd Kuhls

I understand, but removing the gcc version check, which would remove support for xmmintrin.h in all i386 builds, would remove the mmx support for lame on many builds, where it is not necessary.

Yes, understood. Maybe instead add some conditional code in lame to enable the MMX stuff only with known working gcc versions? I think there are some #define you can test to get the gcc version.

Since the work is done in a hook, it can be as simple as:

ifeq ($(BR2_i386),y)
define LAME_COMPILE_FIX

	if $(CC) --version | grep -q '4\.9'; then \
		$(SED) '/xmmintrin\.h/d' $(@D)/configure; \
	fi

endef
endif

However, it should be a post-patch hook, not a pre-configure hook. The
convention is that anything that manipulates the source itself should be done in the patch (or extract) step.

Regards,
Arnout

[snip]

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F