[Buildroot] [PATCH] openblas: fix build issues on ARM

* [Buildroot] [PATCH] openblas: fix build issues on ARM
@ 2016-08-06 21:53 Thomas Petazzoni
  2016-08-07  8:13 ` Khem Raj
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Petazzoni @ 2016-08-06 21:53 UTC (permalink / raw)
  To: buildroot

This commit fixes two build issues of OpenBLAS on ARM:

 - The first one occured on ARMv5 platforms, when the ARMV5 OpenBLAS
   architecture is used. In this case, OpenBLAS build system forces
   -march=armv5, which may not be correct for certain toolchains. As an
   example, the Sourcery CodeBench toolchain has an ARMv4 and an ARMv5
   sysroot. The ARMv5 sysroot is actually an armv5te sysroot, so when
   OpenBLAS forces -march=armv5, gcc thinks it should use the ARMv4
   sysroot, causing build failures.

   To address this, a patch to completely remove the
   architecture-specific CFLAGS is added to OpenBLAS.

   Fixes:

     http://autobuild.buildroot.net/results/991497b12b70f948169e5ad99eebd0fe7f6209a2/

 - The second one occured on ARMv7 platforms, when the ARMV7 OPenBLAS
   architecture is used. In this case, the OpenBLAS build system forces
   -mfloat-abi=hard, which is completely wrong, as using other floating
   point ABIs is perfectly fine.

   To address this, the same patch removing the architecture-specific
   CFLAGS does the job.

   Fixes:

     http://autobuild.buildroot.net/results/0ba0bee48a83367fcefab827e8eaa72f0c8fe90b/

 - Once the previous ARMv7 problem has been fixed, it turns out that the
   ARMv7 specific code in OpenBLAS contains VFPv3 specific
   code. Therefore, the user *must* have choosen either VFPv3 or VFPv4,
   or the code will not build. VFPv3-D16/VFPv4-D16 are not sufficient,
   as more than 16 registers are used by the OpenBLAS code.

   To address this, the ARMV7 platform of OpenBLAS is restricted to the
   proper VFPv3/VFPv4 selection.

   This problem was not visible in the autobuilders, as it was hidden by
   the previous one.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 .../0002-Makefile.arm-remove-contents.patch        | 66 ++++++++++++++++++++++
 package/openblas/Config.in                         |  5 +-
 2 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 package/openblas/0002-Makefile.arm-remove-contents.patch

diff --git a/package/openblas/0002-Makefile.arm-remove-contents.patch b/package/openblas/0002-Makefile.arm-remove-contents.patch
new file mode 100644
index 0000000..06d8e4a
--- /dev/null
+++ b/package/openblas/0002-Makefile.arm-remove-contents.patch
@@ -0,0 +1,66 @@
+From 883e98a1d40d7bb5fd55b80eeddd7dbb79c17835 Mon Sep 17 00:00:00 2001
+From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+Date: Sat, 6 Aug 2016 22:14:22 +0200
+Subject: [PATCH] Makefile.arm: remove contents
+
+The compiler flags being set in Makefile.arm are potentially incorrect
+and harmful:
+
+ - Assuming that one uses -mfloat-abi=hard on ARMv7 is completely
+   wrong. -mfloat-abi=softfp is a perfectly valid choice on ARMv7, and
+   OpenBLAS should not make such assumptions.
+
+ - Assuming that -march=armv5 is correct for ARMv5 is wrong. There are
+   multiple variants: armv5t, armv5te, and using -march=armv5 instead
+   of the proper variant can cause the wrong sysroot to be used in
+   multilib toolchains.
+
+Therefore, the user should simply make sure to provide the relevant
+CFLAGS for his platform, and OpenBLAS should not force them.
+
+Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+---
+ Makefile.arm | 32 +-------------------------------
+ 1 file changed, 1 insertion(+), 31 deletions(-)
+
+diff --git a/Makefile.arm b/Makefile.arm
+index 62bf275..bc63beb 100644
+--- a/Makefile.arm
++++ b/Makefile.arm
+@@ -1,31 +1 @@
+-# ifeq logical or
+-ifeq ($(CORE), $(filter $(CORE),CORTEXA9 CORTEXA15))
+-ifeq ($(OSNAME), Android)
+-CCOMMON_OPT += -marm -mfpu=neon  -mfloat-abi=hard -march=armv7-a
+-FCOMMON_OPT += -marm -mfpu=neon  -mfloat-abi=hard -march=armv7-a
+-else
+-CCOMMON_OPT += -marm -mfpu=vfpv3  -mfloat-abi=hard -march=armv7-a
+-FCOMMON_OPT += -marm -mfpu=vfpv3  -mfloat-abi=hard -march=armv7-a
+-endif
+-endif
+-
+-ifeq ($(CORE), ARMV7)
+-ifeq ($(OSNAME), Android)
+-CCOMMON_OPT += -marm -mfpu=neon  -mfloat-abi=hard -march=armv7-a -Wl,--no-warn-mismatch
+-FCOMMON_OPT += -marm -mfpu=neon  -mfloat-abi=hard -march=armv7-a -Wl,--no-warn-mismatch
+-else
+-CCOMMON_OPT += -marm -mfpu=vfpv3  -mfloat-abi=hard -march=armv7-a
+-FCOMMON_OPT += -marm -mfpu=vfpv3  -mfloat-abi=hard -march=armv7-a
+-endif
+-endif
+-
+-ifeq ($(CORE), ARMV6)
+-CCOMMON_OPT += -marm -mfpu=vfp -mfloat-abi=hard  -march=armv6
+-FCOMMON_OPT += -marm -mfpu=vfp -mfloat-abi=hard  -march=armv6
+-endif
+-
+-
+-ifeq ($(CORE), ARMV5)
+-CCOMMON_OPT += -marm -march=armv5
+-FCOMMON_OPT += -marm -march=armv5
+-endif
++# empty
+\ No newline at end of file
+-- 
+2.7.4
+
diff --git a/package/openblas/Config.in b/package/openblas/Config.in
index 51afaec..a97e935 100644
--- a/package/openblas/Config.in
+++ b/package/openblas/Config.in
@@ -34,7 +34,10 @@ config BR2_PACKAGE_OPENBLAS_DEFAULT_TARGET
 	default "CORTEXA9"     if BR2_cortex_a9
 	default "ARMV5"        if BR2_ARM_CPU_ARMV5
 	default "ARMV6"        if BR2_ARM_CPU_ARMV6
-	default "ARMV7"        if BR2_ARM_CPU_ARMV7A
+	# On ARMv7, OpenBLAS assumes that a full VFPv3 or VFPv4 is
+	# available (and not the more limited D16 variant)
+	default "ARMV7"        if (BR2_ARM_CPU_ARMV7A && \
+				   (BR2_ARM_FPU_VFPV3 || BR2_ARM_FPU_VFPV4))
 	default "ARMV8"        if BR2_aarch64 || BR2_aarch64_be
 	help
 	  OpenBLAS target CPU. See TargetList.txt in the source tree for
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread