Message ID | 20240114135446.1156025-1-fontaine.fabrice@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [v2,1/1] package/tinymembench: force arm mode instead of Thumb mode | expand |
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
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
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
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
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(-)