Message ID | 1471696561-2591-1-git-send-email-thomas.petazzoni@free-electrons.com |
---|---|
State | Accepted |
Commit | 887a1dc3470700db25588affec792d1f5511483a |
Headers | show |
>>>>> "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 --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)
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(+)