diff mbox

openblas: fix build with old binutils versions

Message ID 1471696561-2591-1-git-send-email-thomas.petazzoni@free-electrons.com
State Accepted
Commit 887a1dc3470700db25588affec792d1f5511483a
Headers show

Commit Message

Thomas Petazzoni Aug. 20, 2016, 12:36 p.m. UTC
Older toolchains that use binutils <= 2.23.2 are affected by binutils
bug #14887 (https://sourceware.org/bugzilla/show_bug.cgi?id=14887),
where:

	someinstruction [ foo, something ]

is not accepted, due to the whitespace after [ and before ], causing the
following build failures for OpenBLAS:

  ARM register expected -- `pld [ r1,#512 ]'

Since we don't have any mechanism to add dependencies on binutils
versions, we work around this problem by patching the code to remove the
problematic whitespaces. As there are many many instances of this in the
ARM assembly code of OpenBLAS, we use a sed expression to make this
modification rather than a patch.

Fixes:

  http://autobuild.buildroot.net/results/43e50b480b4aea0fdec745d7875c85377c114cac/

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Since this is a rather non-conventional way of fixing problems, I'll
wait for some people to agree or disagree before applying this
patch. Alternate proposals are welcome.
---
 package/openblas/openblas.mk | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Peter Korsgaard Aug. 22, 2016, 8:10 p.m. UTC | #1
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Older toolchains that use binutils <= 2.23.2 are affected by binutils
 > bug #14887 (https://sourceware.org/bugzilla/show_bug.cgi?id=14887),
 > where:

 > 	someinstruction [ foo, something ]

 > is not accepted, due to the whitespace after [ and before ], causing the
 > following build failures for OpenBLAS:

 >   ARM register expected -- `pld [ r1,#512 ]'

 > Since we don't have any mechanism to add dependencies on binutils
 > versions, we work around this problem by patching the code to remove the
 > problematic whitespaces. As there are many many instances of this in the
 > ARM assembly code of OpenBLAS, we use a sed expression to make this
 > modification rather than a patch.

 > Fixes:

 >   http://autobuild.buildroot.net/results/43e50b480b4aea0fdec745d7875c85377c114cac/

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
 > ---
 > Since this is a rather non-conventional way of fixing problems, I'll
 > wait for some people to agree or disagree before applying this
 > patch. Alternate proposals are welcome.

I also don't really see any other simple way of handling it.

> ---
 >  package/openblas/openblas.mk | 13 +++++++++++++
 >  1 file changed, 13 insertions(+)

 > diff --git a/package/openblas/openblas.mk b/package/openblas/openblas.mk
 > index baeef05..08a2804 100644
 > --- a/package/openblas/openblas.mk
 > +++ b/package/openblas/openblas.mk
 > @@ -38,6 +38,19 @@ else ifeq ($(BR2_SHARED_LIBS),y)
 >  OPENBLAS_MAKE_OPTS += NO_STATIC=1
 >  endif
 
 > +# binutils version <= 2.23.2 have a bug

s/have/has/

> +# (https://sourceware.org/bugzilla/show_bug.cgi?id=14887) where
 > +# whitespaces in ARM register specifications such as [ r1, #12 ] or [
 > +# r2 ] cause the assembler to reject the code. Since there are
 > +# numerous instances of such cases in the code, we use sed rather than
 > +# a patch. We simply replace [ foobar ] by [foobar] to work around the
 > +# problem.
 > +define OPENBLAS_FIXUP_ARM_ASSEMBLY
 > +	$(SED) "s%\[\s*%\[%;s%\s*\]%\]%" $(@D)/kernel/arm/*.S

NIT, we typically use single quotes (') for simplicity in the sed
invocations unless we explicitly need shell expansions.

> +endef
 > +
 > +OPENBLAS_POST_PATCH_HOOKS += OPENBLAS_FIXUP_ARM_ASSEMBLY

I was thinking that this should be inside a BR2_arm / BR2_armeb
conditional, but as it only takes a trivial amount of time to execute,
it is probably nicer to do it unconditionally.

Committed with the above fixed, thanks.
diff mbox

Patch

diff --git a/package/openblas/openblas.mk b/package/openblas/openblas.mk
index baeef05..08a2804 100644
--- a/package/openblas/openblas.mk
+++ b/package/openblas/openblas.mk
@@ -38,6 +38,19 @@  else ifeq ($(BR2_SHARED_LIBS),y)
 OPENBLAS_MAKE_OPTS += NO_STATIC=1
 endif
 
+# binutils version <= 2.23.2 have a bug
+# (https://sourceware.org/bugzilla/show_bug.cgi?id=14887) where
+# whitespaces in ARM register specifications such as [ r1, #12 ] or [
+# r2 ] cause the assembler to reject the code. Since there are
+# numerous instances of such cases in the code, we use sed rather than
+# a patch. We simply replace [ foobar ] by [foobar] to work around the
+# problem.
+define OPENBLAS_FIXUP_ARM_ASSEMBLY
+	$(SED) "s%\[\s*%\[%;s%\s*\]%\]%" $(@D)/kernel/arm/*.S
+endef
+
+OPENBLAS_POST_PATCH_HOOKS += OPENBLAS_FIXUP_ARM_ASSEMBLY
+
 define OPENBLAS_BUILD_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) $(OPENBLAS_MAKE_OPTS) \
 		-C $(@D)