Re: [PATCH 4/4] arm: Enable ARM mode for armv6 strlen

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 13/04/2018 06:56, Phil Blundell wrote:
> On Thu, 2018-04-12 at 17:49 -0300, Adhemerval Zanella wrote:
>> Even though it configure the toolchain to not emit thumb, if user
>> tries to optimize for armv7 (by tinkering with CC or CFLAGS) it will
>> still emit thumb instructions because of the optimized assembly
>> implementations.
>> And the expectation imho is to if user explicit builds with -marm
>> the resulting library should not user thumb instructions.
> 
> I think the precedent on other architectures is that glibc will use the
> instruction set appropriate to the target triple it was given.  For
> example, if you configure glibc for i686-linux-gnu then it will use
> CMOV instructions, and setting -march=i586 in CFLAGS won't prevent
> this.

In fact to determine the sysdeps folders glibc build what machine the
compiler is configured for.  On ARM for instance, the base machine is 
determined at sysdeps/arm/preconfigure.ac by checking the __ARM_ARCH_*
builtin preprocessor direct from compiler. So it does not really matter
if use armv7-linux-gnueabihf is used as triple, since the preprocessor
builtint can be changed by -march.

And I think this is the correct way, the multiarch idea is exactly to 
avoid the necessity of explicit set the target ISA.  And user can also
tune if required by changing the CC/CFLAGS for the desirable target.

> 
> I continue to feel that the scenario mentioned in the bug report you
> linked to (configuring for armv7 but disabling Thumb in the kernel) is
> just silly and we should not be indicating to users that this is
> supported.  T32 is an integral part of ARMv7 (indeed, the M profile
> doesn't support A32 at all) and if you take it out then the resulting
> architecture is no longer ARMv7.  It seems undesirable to force all the
> ARMv7-optimised assembly routines to also provide an ARM-only version
> even if the resulting performance is the same or worse than the Thumb
> implementation.  This code is never going to get tested in practice
> (witness the fact that you found it's been broken for several years)
> and it's just a liability.

Since T32 is an integral part of ARMv7, I would expect either that kernel
do not provide an option to disable it or at least emulate thumb instruction
if underlying hardware do not provide it (as for other various
architectures for some atomic operation for instance). However the current 
scenario exists and I see no strong reason to not support it. 

We already handle the cases in generic code where we should not generate
thumb (NO_THUMB macro) and the fixes I sent already are minimal.  For
newer implementation it is a matter of if the idea is to always support
thumb so simple redirection to a previous implementation is straightforward
(ifndef __thumb__ then include old implementation).

But I give you this incurs in more maintainability and I do agree we should
avoid such path. However what we shouldn't is simply breaking at runtime
due a non-supported configuration. If the idea is to support thumb as default,
we should then indicate at build time that it is required (either at configure
time or at build time).

> 
>> In fact, the whole idea of current code is indeed to prevent thumb
>> instructions in such cases
> 
> That's true.  It's not entirely clear to me why Roland made that change
>  in the first place but I think it was roughly contemporaneous with the
> NaCl port and I'm sort of guessing it was something to do with that. 
> Does anybody else know/remember?
> 
> p.
>