[PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Stefan Ghinea]

From: Catalin Enache <catalin.enache@windriver.com>

When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
since the Thumb frame pointer in r7 clashes with pulseaudio's use of inline
asm to make syscalls (where r7 is used for the syscall NR).

In most cases, frame pointers will be disabled automatically due to
the optimisation level, but appending an explicit -fomit-frame-pointer
to CFLAGS handles cases where optimisation is set to -O0 or frame
pointers have been enabled by -fno-omit-frame-pointer earlier in
CFLAGS, etc.

References:
https://www.openwall.com/lists/musl/2017/10/09/2

Signed-off-by: Catalin Enache <catalin.enache@windriver.com>
Signed-off-by: Stefan Ghinea <stefan.ghinea@windriver.com>
---
 meta/recipes-multimedia/pulseaudio/pulseaudio.inc | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
index 4e32b27087..c7f3e67022 100644
--- a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
+++ b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
@@ -63,6 +63,14 @@ DEPENDS += "speexdsp libxml-parser-perl-native libcap"
 
 inherit autotools bash-completion pkgconfig useradd gettext perlnative systemd manpages gsettings
 
+# When compiling for Thumb or Thumb2, frame pointers _must_ be disabled since the
+# Thumb frame pointer in r7 clashes with pulseaudio's use of inline asm to make syscalls
+# (where r7 is used for the syscall NR). In most cases, frame pointers will be
+# disabled automatically due to the optimisation level, but append an explicit
+# -fomit-frame-pointer to handle cases where optimisation is set to -O0 or frame
+# pointers have been enabled by -fno-omit-frame-pointer earlier in CFLAGS, etc.
+CFLAGS_append_arm = " ${@bb.utils.contains('TUNE_CCARGS', '-mthumb', '-fomit-frame-pointer', '', d)}"
+
 # *.desktop rules wont be generated during configure and build will fail
 # if using --disable-nls
 USE_NLS = "yes"
-- 
2.17.1

Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Andre McCurdy]

On Thu, Mar 26, 2020 at 8:26 AM Stefan Ghinea
<stefan.ghinea@windriver.com> wrote:
>
> From: Catalin Enache <catalin.enache@windriver.com>
>
> When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
> since the Thumb frame pointer in r7 clashes with pulseaudio's use of inline
> asm to make syscalls (where r7 is used for the syscall NR).
>
> In most cases, frame pointers will be disabled automatically due to
> the optimisation level, but appending an explicit -fomit-frame-pointer
> to CFLAGS handles cases where optimisation is set to -O0 or frame
> pointers have been enabled by -fno-omit-frame-pointer earlier in
> CFLAGS, etc.
>
> References:
> https://www.openwall.com/lists/musl/2017/10/09/2
>
> Signed-off-by: Catalin Enache <catalin.enache@windriver.com>
> Signed-off-by: Stefan Ghinea <stefan.ghinea@windriver.com>
> ---
>  meta/recipes-multimedia/pulseaudio/pulseaudio.inc | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
> index 4e32b27087..c7f3e67022 100644
> --- a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
> +++ b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
> @@ -63,6 +63,14 @@ DEPENDS += "speexdsp libxml-parser-perl-native libcap"
>
>  inherit autotools bash-completion pkgconfig useradd gettext perlnative systemd manpages gsettings
>
> +# When compiling for Thumb or Thumb2, frame pointers _must_ be disabled since the
> +# Thumb frame pointer in r7 clashes with pulseaudio's use of inline asm to make syscalls
> +# (where r7 is used for the syscall NR). In most cases, frame pointers will be
> +# disabled automatically due to the optimisation level, but append an explicit
> +# -fomit-frame-pointer to handle cases where optimisation is set to -O0 or frame
> +# pointers have been enabled by -fno-omit-frame-pointer earlier in CFLAGS, etc.
> +CFLAGS_append_arm = " ${@bb.utils.contains('TUNE_CCARGS', '-mthumb', '-fomit-frame-pointer', '', d)}"

This is OK and safe to merge but note that it's now regarded as the
"old style" fix for this problem. The newer approach is to tweak the
asm code to save the frame pointer before making the syscall and
restore it afterwards. e.g.

  https://github.com/strace/strace/commit/0c75ebaad09d6d3f2395dfe6160904af883dd0d9

If pulseaudio can be fixed in the same way then that's probably a
better approach.

>  # *.desktop rules wont be generated during configure and build will fail
>  # if using --disable-nls
>  USE_NLS = "yes"
> --
> 2.17.1

Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Adrian Bunk]

On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote:
>...
> When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
> since the Thumb frame pointer in r7
>...

How are you reproducing the problem in pulseaudio?

This sounds like a workaround for a bug in musl that was fixed 2 years ago.

cu
Adrian

Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Andre McCurdy]

On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote:
>
> On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote:
> >...
> > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
> > since the Thumb frame pointer in r7
> >...
>
> How are you reproducing the problem in pulseaudio?
>
> This sounds like a workaround for a bug in musl that was fixed 2 years ago.

The problem can show up anywhere that inline asm is trying to use r7.
In this case it looks like:

  https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50

Better fix there might be to use registers { r4,r5,r6,r12 } instead of
{ r4,r5,r6,r7 } ?

> cu
> Adrian
> 

Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Adrian Bunk]

On Thu, Mar 26, 2020 at 12:53:08PM -0700, Andre McCurdy wrote:
> On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote:
> >
> > On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote:
> > >...
> > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
> > > since the Thumb frame pointer in r7
> > >...
> >
> > How are you reproducing the problem in pulseaudio?
> >
> > This sounds like a workaround for a bug in musl that was fixed 2 years ago.
> 
> The problem can show up anywhere that inline asm is trying to use r7.
> In this case it looks like:
> 
>   https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50
>...

After looking at the pulseaudio code I suspected the patch description 
claiming pulseaudio syscall code would be the problem was rubbish, and 
that this NEON code was the problem.

But when I tried to reproduce the problem it built for me with both 
glibc and musl in master (the patch didn't mention that this was a
musl-only problem).

Then I saw that this was fixed in musl upstream 2 years ago:
https://git.musl-libc.org/cgit/musl/commit/?id=e3c682ab5257aaa6739ef242a9676d897370e78e

cu
Adrian

Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Andre McCurdy]

On Thu, Mar 26, 2020 at 1:26 PM Adrian Bunk <bunk@stusta.de> wrote:
>
> On Thu, Mar 26, 2020 at 12:53:08PM -0700, Andre McCurdy wrote:
> > On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote:
> > > >...
> > > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
> > > > since the Thumb frame pointer in r7
> > > >...
> > >
> > > How are you reproducing the problem in pulseaudio?
> > >
> > > This sounds like a workaround for a bug in musl that was fixed 2 years ago.
> >
> > The problem can show up anywhere that inline asm is trying to use r7.
> > In this case it looks like:
> >
> >   https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50
> >...
>
> After looking at the pulseaudio code I suspected the patch description
> claiming pulseaudio syscall code would be the problem was rubbish, and
> that this NEON code was the problem.

Yes, the comment looks like it was copied and pasted and doesn't
really apply in this case (since pulseaudio isn't making syscalls).
That should be updated.

> But when I tried to reproduce the problem it built for me with both
> glibc and musl in master (the patch didn't mention that this was a
> musl-only problem).
>
> Then I saw that this was fixed in musl upstream 2 years ago:
> https://git.musl-libc.org/cgit/musl/commit/?id=e3c682ab5257aaa6739ef242a9676d897370e78e

Right, it's not related to musl or glibc. I suspect it can be
reproduced by building for an ARM target which supports NEON, ensuring
that DEFAULTTUNE doesn't forcefully disable Thumb (e.g. it should be
armv7vethf-neon, not armv7vehf-neon), setting ARM_INSTRUCTION_SET to
thumb and then compiling with frame pointers enabled (e.g. by adding
-fno-omit-frame-pointer to CFLAGS).

In terms of a fix, then changing the code to use r12 instead of r7 is
probably the best solution (assuming it works), but would need careful
testing. Appending -fomit-frame-pointer to CFLAGS for ARM machines
building for Thumb is safe and should fix the issue too. Presumably
limiting the -fomit-frame-pointer workaround to ARM machines which
support NEON building for Thumb would be an even more targeted
solution.

Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Adrian Bunk]

On Thu, Mar 26, 2020 at 02:23:03PM -0700, Andre McCurdy wrote:
>...
> Right, it's not related to musl or glibc. I suspect it can be
> reproduced by building for an ARM target which supports NEON, ensuring
> that DEFAULTTUNE doesn't forcefully disable Thumb (e.g. it should be
> armv7vethf-neon, not armv7vehf-neon), setting ARM_INSTRUCTION_SET to
> thumb and then compiling with frame pointers enabled (e.g. by adding
> -fno-omit-frame-pointer to CFLAGS).
>...

Thanks, this was helpful in figuring out what I missed when testing.
I can now confirm that the problem still exists, also with glibc.

cu
Adrian

Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Tanu Kaskinen]

On Thu, 2020-03-26 at 17:26 +0200, Stefan Ghinea wrote:
> From: Catalin Enache <catalin.enache@windriver.com>
> 
> When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
> since the Thumb frame pointer in r7 clashes with pulseaudio's use of inline
> asm to make syscalls (where r7 is used for the syscall NR).

Do you have a pointer to the asm code that does syscalls? I'm an
upstream maintainer of PulseAudio, and to me it sounds a bit strange
that there would be inline asm that does syscalls.

It would be great to fix this problem in upstream (either by applying
this fix/workaround in the build system, or by modifying the asm code
so that the problem goes away).

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk

Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Tanu Kaskinen]

On Mon, 2020-03-30 at 19:29 +0300, Tanu Kaskinen wrote:
> On Thu, 2020-03-26 at 17:26 +0200, Stefan Ghinea wrote:
> > From: Catalin Enache <catalin.enache@windriver.com>
> > 
> > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
> > since the Thumb frame pointer in r7 clashes with pulseaudio's use of inline
> > asm to make syscalls (where r7 is used for the syscall NR).
> 
> Do you have a pointer to the asm code that does syscalls? I'm an
> upstream maintainer of PulseAudio, and to me it sounds a bit strange
> that there would be inline asm that does syscalls.
> 
> It would be great to fix this problem in upstream (either by applying
> this fix/workaround in the build system, or by modifying the asm code
> so that the problem goes away).

I forgot to check if others had already commented on this... So the
real problem is that src/pulsecore/remap_neon.c uses the r7 register in
a few places. Andre McCurdy said: "In terms of a fix, then changing the
code to use r12 instead of r7 is probably the best solution (assuming
it works), but would need careful testing." I'll try this.

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk

Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Tanu Kaskinen]

On Thu, 2020-03-26 at 14:23 -0700, Andre McCurdy wrote:
> On Thu, Mar 26, 2020 at 1:26 PM Adrian Bunk <bunk@stusta.de> wrote:
> > On Thu, Mar 26, 2020 at 12:53:08PM -0700, Andre McCurdy wrote:
> > > On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote:
> > > > On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote:
> > > > > ...
> > > > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
> > > > > since the Thumb frame pointer in r7
> > > > > ...
> > > > 
> > > > How are you reproducing the problem in pulseaudio?
> > > > 
> > > > This sounds like a workaround for a bug in musl that was fixed 2 years ago.
> > > 
> > > The problem can show up anywhere that inline asm is trying to use r7.
> > > In this case it looks like:
> > > 
> > >   https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50
> > > ...
> > 
> > After looking at the pulseaudio code I suspected the patch description
> > claiming pulseaudio syscall code would be the problem was rubbish, and
> > that this NEON code was the problem.
> 
> Yes, the comment looks like it was copied and pasted and doesn't
> really apply in this case (since pulseaudio isn't making syscalls).
> That should be updated.
> 
> > But when I tried to reproduce the problem it built for me with both
> > glibc and musl in master (the patch didn't mention that this was a
> > musl-only problem).
> > 
> > Then I saw that this was fixed in musl upstream 2 years ago:
> > https://git.musl-libc.org/cgit/musl/commit/?id=e3c682ab5257aaa6739ef242a9676d897370e78e
> 
> Right, it's not related to musl or glibc. I suspect it can be
> reproduced by building for an ARM target which supports NEON, ensuring
> that DEFAULTTUNE doesn't forcefully disable Thumb (e.g. it should be
> armv7vethf-neon, not armv7vehf-neon), setting ARM_INSTRUCTION_SET to
> thumb and then compiling with frame pointers enabled (e.g. by adding
> -fno-omit-frame-pointer to CFLAGS).
> 
> In terms of a fix, then changing the code to use r12 instead of r7 is
> probably the best solution (assuming it works), but would need careful
> testing. Appending -fomit-frame-pointer to CFLAGS for ARM machines
> building for Thumb is safe and should fix the issue too. Presumably
> limiting the -fomit-frame-pointer workaround to ARM machines which
> support NEON building for Thumb would be an even more targeted
> solution.

I finally found time to test fixing the assembly code to use r12
instead of r7. Seems to work fine (I was first baffled by incorrect
behaviour, because I changed "{r4-r7}" to "{r4-r12}" without realizing
that "r4-r12" meant a range of all registers from r4 to r12).

Can you enlighten me: why did you choose r12 instead of r8? Why did the
original author use registers r4-r7 instead of r0-r3? Is it somehow
advisable to avoid registers r0-r3 and r8-r11? The code seems to work
fine with any set of registers, except r7.

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk

Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Andre McCurdy]

On Fri, Jul 17, 2020 at 3:09 AM Tanu Kaskinen <tanuk@iki.fi> wrote:
> On Thu, 2020-03-26 at 14:23 -0700, Andre McCurdy wrote:
> > On Thu, Mar 26, 2020 at 1:26 PM Adrian Bunk <bunk@stusta.de> wrote:
> > > On Thu, Mar 26, 2020 at 12:53:08PM -0700, Andre McCurdy wrote:
> > > > On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote:
> > > > > On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote:
> > > > > > ...
> > > > > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
> > > > > > since the Thumb frame pointer in r7
> > > > > > ...
> > > > >
> > > > > How are you reproducing the problem in pulseaudio?
> > > > >
> > > > > This sounds like a workaround for a bug in musl that was fixed 2 years ago.
> > > >
> > > > The problem can show up anywhere that inline asm is trying to use r7.
> > > > In this case it looks like:
> > > >
> > > >   https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50
> > > > ...
> > >
> > > After looking at the pulseaudio code I suspected the patch description
> > > claiming pulseaudio syscall code would be the problem was rubbish, and
> > > that this NEON code was the problem.
> >
> > Yes, the comment looks like it was copied and pasted and doesn't
> > really apply in this case (since pulseaudio isn't making syscalls).
> > That should be updated.
> >
> > > But when I tried to reproduce the problem it built for me with both
> > > glibc and musl in master (the patch didn't mention that this was a
> > > musl-only problem).
> > >
> > > Then I saw that this was fixed in musl upstream 2 years ago:
> > > https://git.musl-libc.org/cgit/musl/commit/?id=e3c682ab5257aaa6739ef242a9676d897370e78e
> >
> > Right, it's not related to musl or glibc. I suspect it can be
> > reproduced by building for an ARM target which supports NEON, ensuring
> > that DEFAULTTUNE doesn't forcefully disable Thumb (e.g. it should be
> > armv7vethf-neon, not armv7vehf-neon), setting ARM_INSTRUCTION_SET to
> > thumb and then compiling with frame pointers enabled (e.g. by adding
> > -fno-omit-frame-pointer to CFLAGS).
> >
> > In terms of a fix, then changing the code to use r12 instead of r7 is
> > probably the best solution (assuming it works), but would need careful
> > testing. Appending -fomit-frame-pointer to CFLAGS for ARM machines
> > building for Thumb is safe and should fix the issue too. Presumably
> > limiting the -fomit-frame-pointer workaround to ARM machines which
> > support NEON building for Thumb would be an even more targeted
> > solution.
>
> I finally found time to test fixing the assembly code to use r12
> instead of r7. Seems to work fine (I was first baffled by incorrect
> behaviour, because I changed "{r4-r7}" to "{r4-r12}" without realizing
> that "r4-r12" meant a range of all registers from r4 to r12).
>
> Can you enlighten me: why did you choose r12 instead of r8? Why did the
> original author use registers r4-r7 instead of r0-r3? Is it somehow
> advisable to avoid registers r0-r3 and r8-r11? The code seems to work
> fine with any set of registers, except r7.

The compiler will work around whichever set of registers you want to
use (apart from r7 in some cases) so it's expected that other
combinations will work fine. Some combinations will be more efficient
than others (ie the compiler will need to do less shuffling values
between registers or saving register values to the stack in order to
make registers you specify available to you). Using r12 instead of r8
is just an educated guess about the combination will allow the
compiler to generate the most efficient code.

Registers r0-r3 and r12 can be used within a function without needing
to preserve their previous contents, so if a function needs registers
it's more efficient to use these registers first. Other registers need
to be preserved (ie saved to the stack) before use.

Registers r0-r3 are also used to pass non-floating point arguments to
a function, so if a function takes 4 or more non-floating point
arguments, then r0-r3 will already contain values which the function
will need to use.

Note that in this particular function, the first argument (ie the
pointer m) is never actually used, so it may be that using r0, r4, r5
and r12 will give the best result. The function is pretty trivial
though so I guess with a recent compiler just writing the whole thing
in C will give close to the optimal result too without all the
maintenance issues.

Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error

[Tanu Kaskinen]

On Fri, 2020-07-17 at 12:19 -0700, Andre McCurdy wrote:
> On Fri, Jul 17, 2020 at 3:09 AM Tanu Kaskinen <tanuk@iki.fi> wrote:
> > On Thu, 2020-03-26 at 14:23 -0700, Andre McCurdy wrote:
> > > On Thu, Mar 26, 2020 at 1:26 PM Adrian Bunk <bunk@stusta.de> wrote:
> > > > On Thu, Mar 26, 2020 at 12:53:08PM -0700, Andre McCurdy wrote:
> > > > > On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote:
> > > > > > On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote:
> > > > > > > ...
> > > > > > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
> > > > > > > since the Thumb frame pointer in r7
> > > > > > > ...
> > > > > > 
> > > > > > How are you reproducing the problem in pulseaudio?
> > > > > > 
> > > > > > This sounds like a workaround for a bug in musl that was fixed 2 years ago.
> > > > > 
> > > > > The problem can show up anywhere that inline asm is trying to use r7.
> > > > > In this case it looks like:
> > > > > 
> > > > >   https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50
> > > > > ...
> > > > 
> > > > After looking at the pulseaudio code I suspected the patch description
> > > > claiming pulseaudio syscall code would be the problem was rubbish, and
> > > > that this NEON code was the problem.
> > > 
> > > Yes, the comment looks like it was copied and pasted and doesn't
> > > really apply in this case (since pulseaudio isn't making syscalls).
> > > That should be updated.
> > > 
> > > > But when I tried to reproduce the problem it built for me with both
> > > > glibc and musl in master (the patch didn't mention that this was a
> > > > musl-only problem).
> > > > 
> > > > Then I saw that this was fixed in musl upstream 2 years ago:
> > > > https://git.musl-libc.org/cgit/musl/commit/?id=e3c682ab5257aaa6739ef242a9676d897370e78e
> > > 
> > > Right, it's not related to musl or glibc. I suspect it can be
> > > reproduced by building for an ARM target which supports NEON, ensuring
> > > that DEFAULTTUNE doesn't forcefully disable Thumb (e.g. it should be
> > > armv7vethf-neon, not armv7vehf-neon), setting ARM_INSTRUCTION_SET to
> > > thumb and then compiling with frame pointers enabled (e.g. by adding
> > > -fno-omit-frame-pointer to CFLAGS).
> > > 
> > > In terms of a fix, then changing the code to use r12 instead of r7 is
> > > probably the best solution (assuming it works), but would need careful
> > > testing. Appending -fomit-frame-pointer to CFLAGS for ARM machines
> > > building for Thumb is safe and should fix the issue too. Presumably
> > > limiting the -fomit-frame-pointer workaround to ARM machines which
> > > support NEON building for Thumb would be an even more targeted
> > > solution.
> > 
> > I finally found time to test fixing the assembly code to use r12
> > instead of r7. Seems to work fine (I was first baffled by incorrect
> > behaviour, because I changed "{r4-r7}" to "{r4-r12}" without realizing
> > that "r4-r12" meant a range of all registers from r4 to r12).
> > 
> > Can you enlighten me: why did you choose r12 instead of r8? Why did the
> > original author use registers r4-r7 instead of r0-r3? Is it somehow
> > advisable to avoid registers r0-r3 and r8-r11? The code seems to work
> > fine with any set of registers, except r7.
> 
> The compiler will work around whichever set of registers you want to
> use (apart from r7 in some cases) so it's expected that other
> combinations will work fine. Some combinations will be more efficient
> than others (ie the compiler will need to do less shuffling values
> between registers or saving register values to the stack in order to
> make registers you specify available to you). Using r12 instead of r8
> is just an educated guess about the combination will allow the
> compiler to generate the most efficient code.
> 
> Registers r0-r3 and r12 can be used within a function without needing
> to preserve their previous contents, so if a function needs registers
> it's more efficient to use these registers first. Other registers need
> to be preserved (ie saved to the stack) before use.
> 
> Registers r0-r3 are also used to pass non-floating point arguments to
> a function, so if a function takes 4 or more non-floating point
> arguments, then r0-r3 will already contain values which the function
> will need to use.
> 
> Note that in this particular function, the first argument (ie the
> pointer m) is never actually used, so it may be that using r0, r4, r5
> and r12 will give the best result. The function is pretty trivial
> though so I guess with a recent compiler just writing the whole thing
> in C will give close to the optimal result too without all the
> maintenance issues.

Thanks for explaining!

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk