Re: [Buildroot] [PATCH 2/3] package/poke: disable minimal-threading with thumb or thumb2

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello,

On Sat, 29 Jan 2022 15:23:51 +0100
Romain Naour <romain.naour@gmail.com> wrote:

> +# Disable minimal-threading when thumb or thumb2 is enabled
> +# due to runtime issue.
> +# https://lists.gnu.org/archive/html/poke-devel/2022-01/msg00162.html
> +ifeq ($(BR2_ARM_CPU_HAS_THUMB)$(BR2_ARM_CPU_HAS_THUMB2),y)
> +POKE_CONF_OPTS += --disable-dispatch-minimal-threading
> +else
> +POKE_CONF_OPTS += --enable-dispatch-minimal-threading
> +endif

I don't understand this patch, because minimal-threading is already
disabled by default (at least in poke 1.4, I haven't checked in poke
2.0). jitter/configure.ac contains:

# Is the minimal-threading dispatch enabled?  Check the default and the
# configure command-line option.
AC_MSG_CHECKING([if minimal-threading dispatch is enabled])
# FIXME: reenable this when I make defect handling reliable.
#default=$(test "x$JITTER_HAVE_ACTUAL_GCC" = "xyes" && echo yes || echo no)
default=no
AC_ARG_ENABLE([dispatch-minimal-threading],
              AS_HELP_STRING([--enable-dispatch-minimal-threading],
              [enable minimal threading dispatch: default yes if and
only if GCC (not an imitation) is used]),
              jitter_enable_dispatch_minimal_threading="$enableval",
              jitter_enable_dispatch_minimal_threading="$default")
if test "x$jitter_enable_dispatch_minimal_threading" = "xyes"; then
  jitter_best_dispatch_model="minimal-threading"
  jitter_enabled_dispatch_models="$jitter_enabled_dispatch_models minimal-threading"
fi
AC_MSG_RESULT([$jitter_enable_dispatch_minimal_threading$jitter_subpackage_disclaimer])
# FIXME: remove this warning message when I make defect handling reliable.
if test "x$jitter_enable_dispatch_minimal_threading" = "xyes"; then
  AC_MSG_WARN([minimal-threading dispatch is currently unreliable, and should
  not be used in production until defective instruction replacement is fully
  implemented])
fi

See the "default=no" ?

Also, when building with the following defconfig:

BR2_arm=y
BR2_cortex_a8=y
BR2_ARM_EABI=y
BR2_ARM_INSTRUCTIONS_THUMB2=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_POKE=y
# BR2_TARGET_ROOTFS_TAR is not set

The minimal threading is already disabled:

checking if switch dispatching is enabled... yes, provisionally (sub-package mode)
checking if direct-threading dispatch is enabled... yes, provisionally (sub-package mode)
checking if minimal-threading dispatch is enabled... no, provisionally (sub-package mode)
checking if no-threading dispatch is enabled... no, provisionally (sub-package mode)
configure: sub-package mode: disabling every dispatch except
direct-threading
configure:   (the following dispatches could have been enabled:
  switch direct-threading)
configure: enabled dispatching models are:
  direct-threading
configure: the best enabled dispatching model is:
  direct-threading

So what this patch is doing is in fact slightly the opposite of what he
says: instead of disabling minimal threading for thumb or thumb2, it
enables it for any other platform.

Are you sure this patch was needed for poke 1.4, and not for poke 2.0 ?

Best regards,

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