[Buildroot] [PATCH v2, 1/1] package/tinymembench: force arm mode instead of Thumb mode

[Buildroot] [PATCH v2, 1/1] package/tinymembench: force arm mode instead of Thumb mode

[Fabrice Fontaine]

Fix the following build failure in Thumb mode:

/tmp/ccaZHrla.s:40: Error: instruction not supported in Thumb16 mode -- `subs r1,r1,#16'
/tmp/ccaZHrla.s:43: Error: instruction not supported in Thumb16 mode -- `subs r1,r1,#16'
main.c:45: Error: selected processor does not support `mla r2,r10,r2,r5' in Thumb mode
main.c:46: Error: unshifted register required -- `and r8,r7,r2,lsr#16'
main.c:47: Error: selected processor does not support `mla r2,r10,r2,r5' in Thumb mode
main.c:48: Error: unshifted register required -- `and r9,r6,r2,lsr#8'
main.c:49: Error: selected processor does not support `mla r2,r10,r2,r5' in Thumb mode

While at it, add upstream tag to first patch

Fixes:
 - http://autobuild.buildroot.org/results/1e359c294a8d71fb1833e5d04a6bc7d4fd533510
 - http://autobuild.buildroot.org/results/c06010d7a2bdb33a1707266133a3880e14be7657

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
Changes v1 -> v2 (after review of Yann E. Morin):
 - Keep patch

 .checkpackageignore                                      | 1 -
 .../0001-arm-fix-build-on-Thumb-only-architectures.patch | 1 +
 package/tinymembench/tinymembench.mk                     | 9 ++++++++-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/.checkpackageignore b/.checkpackageignore
index 4051805c33..09a1bf35e0 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -1296,7 +1296,6 @@ package/tinyalsa/0001-include-time.h-before-asound.h.patch Upstream
 package/tinycbor/0001-Makefile-add-DISABLE_WERROR.patch Upstream
 package/tinycompress/0001-wave-add-time.h-missing-header-inclusion.patch Upstream
 package/tinydtls/0001-sha2-sha2.c-fix-build-on-big-endian.patch Upstream
-package/tinymembench/0001-arm-fix-build-on-Thumb-only-architectures.patch Upstream
 package/tinyproxy/0001-prevent-junk-from-showing-up-in-error-page-in-invalid-requests.patch Upstream
 package/tinyxml/0001-In-stamp-always-advance-the-pointer-if-p-0xef.patch Upstream
 package/tpm2-abrmd/S80tpm2-abrmd Indent Shellcheck Variables
diff --git a/package/tinymembench/0001-arm-fix-build-on-Thumb-only-architectures.patch b/package/tinymembench/0001-arm-fix-build-on-Thumb-only-architectures.patch
index 88559ec012..cefa4e995f 100644
--- a/package/tinymembench/0001-arm-fix-build-on-Thumb-only-architectures.patch
+++ b/package/tinymembench/0001-arm-fix-build-on-Thumb-only-architectures.patch
@@ -11,6 +11,7 @@ architecture supports the ARM instruction set, by testing the
 __ARM_ARCH_ISA_ARM compiler define.
 
 Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+Upstream: https://github.com/ssvb/tinymembench/pull/13
 ---
  arm-neon.S | 2 +-
  asm-opt.c  | 2 +-
diff --git a/package/tinymembench/tinymembench.mk b/package/tinymembench/tinymembench.mk
index 016d680ce7..bc84595e24 100644
--- a/package/tinymembench/tinymembench.mk
+++ b/package/tinymembench/tinymembench.mk
@@ -9,8 +9,15 @@ TINYMEMBENCH_SITE = $(call github,ssvb,tinymembench,v$(TINYMEMBENCH_VERSION))
 TINYMEMBENCH_LICENSE = MIT
 TINYMEMBENCH_LICENSE_FILES = LICENSE
 
+# tinymembench has some assembly function that is not present in Thumb mode:
+# Error: instruction not supported in Thumb16 mode -- `subs r1,r1,#16'
+# so, we desactivate Thumb mode
+ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB),y)
+TINYMEMBENCH_MAKE_OPTS += CFLAGS="$(TARGET_CFLAGS) -marm"
+endif
+
 define TINYMEMBENCH_BUILD_CMDS
-	$(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D)
+	$(TARGET_CONFIGURE_OPTS) $(MAKE) $(TINYMEMBENCH_MAKE_OPTS) -C $(@D)
 endef
 
 define TINYMEMBENCH_INSTALL_TARGET_CMDS
-- 
2.43.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2, 1/1] package/tinymembench: force arm mode instead of Thumb mode

[Thomas Petazzoni via buildroot]

Hello Fabrice,

On Sun, 14 Jan 2024 14:54:46 +0100
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> +# tinymembench has some assembly function that is not present in Thumb mode:
> +# Error: instruction not supported in Thumb16 mode -- `subs r1,r1,#16'
> +# so, we desactivate Thumb mode
> +ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB),y)
> +TINYMEMBENCH_MAKE_OPTS += CFLAGS="$(TARGET_CFLAGS) -marm"
> +endif

Why don't we simply extend the existing patch in the same way? It's
just that new code has been added in main.c that isn't Thumb
compatible, so I think you could change:

#ifdef __arm__

to

#if defined(__arm__) && defined(__ARM_ARCH_ISA_ARM)

in main.c, and this should get you going. Could you try this instead?

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2, 1/1] package/tinymembench: force arm mode instead of Thumb mode

[Fabrice Fontaine]

Hello,

Le mer. 7 févr. 2024 à 16:47, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a écrit :
>
> Hello Fabrice,
>
> On Sun, 14 Jan 2024 14:54:46 +0100
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > +# tinymembench has some assembly function that is not present in Thumb mode:
> > +# Error: instruction not supported in Thumb16 mode -- `subs r1,r1,#16'
> > +# so, we desactivate Thumb mode
> > +ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB),y)
> > +TINYMEMBENCH_MAKE_OPTS += CFLAGS="$(TARGET_CFLAGS) -marm"
> > +endif
>
> Why don't we simply extend the existing patch in the same way? It's
> just that new code has been added in main.c that isn't Thumb
> compatible, so I think you could change:
>
> #ifdef __arm__
>
> to
>
> #if defined(__arm__) && defined(__ARM_ARCH_ISA_ARM)
>
> in main.c, and this should get you going. Could you try this instead?

First, it should be noted that the build failure is not related to any
new code, this code was already there in v0.3:
https://github.com/ssvb/tinymembench/blob/v0.3/main.c

Moreover, I already tried that approach and this doesn't work as
__ARM_ARCH_ISA_ARM is defined by the toolchain.
__ARM_ARCH_ISA_THUMB value could be checked instead but I was not
confident in this solution as:
 - I don't fully understand what are the different meaning of
__ARM_ARCH_ISA_{ARM,THUMB} values
 - upstream feedback on first patch was not really positive back in 2017
 - upstream seems dead now so non upstreamable patches will have to be
kept "forever"

>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com

Best Regards,

Fabrice
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2, 1/1] package/tinymembench: force arm mode instead of Thumb mode

[Thomas Petazzoni via buildroot]

On Wed, 7 Feb 2024 21:48:51 +0100
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

>  - upstream seems dead now so non upstreamable patches will have to be
> kept "forever"

Indeed. The repo has not seen any commits since 11 years, despite PRs
and issues being opened by several people. I think we should drop this
package, and stop bothering.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot