[Buildroot] [PATCH] libnspr: only enable thumb if BR2_ARM_INSTRUCTIONS_THUMB2

[Buildroot] [PATCH] libnspr: only enable thumb if BR2_ARM_INSTRUCTIONS_THUMB2

[Arnout Vandecappelle]

libnspr currently passes --enable-thumb2 if the CPU has thumb
instructions. This option will pass -mthumb to the compiler. However,
if an external multilib toolchain is used that has a thumb-specific
variant, it will try to use that one. But we only copy a single variant
to the sysroot, so the build will fail with:

In order to fix this, only enable thumb2 when we are really using
thumb2. Note that it is still necessary to pass this option, cfr.
http://autobuild.buildroot.org/results/d7323831372050e425a34f5104a46d8cbd6be214

We could have chosen to still enable thumb2 with the internal toolchain.
However, that would mean we create a discrepancy between the result of
building with the same (buildroot) toolchain when it is used as an
internal one or as an external one. To avoid potentially surprising
results, just stick to the option chosen for the rest of the rootfs.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Build-tested with various combinations of thumb, thumb2, floating point,
and external toolchains. I didn't test with an internal toolchain but
what could go wrong there? :-)
---
 package/libnspr/libnspr.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/libnspr/libnspr.mk b/package/libnspr/libnspr.mk
index 8e58986..c9bfa0c 100644
--- a/package/libnspr/libnspr.mk
+++ b/package/libnspr/libnspr.mk
@@ -50,7 +50,7 @@ LIBNSPR_INSTALL_STAGING_OPTS = DESTDIR=$(STAGING_DIR) LIBRARY= install
 endif
 
 ifeq ($(BR2_arm),y)
-ifeq ($(BR2_ARM_CPU_HAS_THUMB2),y)
+ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB2),y)
 LIBNSPR_CONF_OPTS += --enable-thumb2
 else
 LIBNSPR_CONF_OPTS += --disable-thumb2
-- 
2.7.0.rc3

[Buildroot] [PATCH] libnspr: only enable thumb if BR2_ARM_INSTRUCTIONS_THUMB2

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> writes:

 > libnspr currently passes --enable-thumb2 if the CPU has thumb
 > instructions. This option will pass -mthumb to the compiler. However,
 > if an external multilib toolchain is used that has a thumb-specific
 > variant, it will try to use that one. But we only copy a single variant
 > to the sysroot, so the build will fail with:

 > In order to fix this, only enable thumb2 when we are really using
 > thumb2. Note that it is still necessary to pass this option, cfr.
 > http://autobuild.buildroot.org/results/d7323831372050e425a34f5104a46d8cbd6be214

 > We could have chosen to still enable thumb2 with the internal toolchain.
 > However, that would mean we create a discrepancy between the result of
 > building with the same (buildroot) toolchain when it is used as an
 > internal one or as an external one. To avoid potentially surprising
 > results, just stick to the option chosen for the rest of the rootfs.

Why do we actually ever pass --enable-thumb2 / --disable-thumb2? As far
as I can see, the only thing this does is adding -marm / -mthumb to
CFLAGS, and we already pass the correct -m<arm/thumb> option in the
toolchain wrapper.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] libnspr: only enable thumb if BR2_ARM_INSTRUCTIONS_THUMB2

[Arnout Vandecappelle]

On 15-01-16 21:43, Peter Korsgaard wrote:
>>>>>> "Arnout" == Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> writes:
> 
>  > libnspr currently passes --enable-thumb2 if the CPU has thumb
>  > instructions. This option will pass -mthumb to the compiler. However,
>  > if an external multilib toolchain is used that has a thumb-specific
>  > variant, it will try to use that one. But we only copy a single variant
>  > to the sysroot, so the build will fail with:
> 
>  > In order to fix this, only enable thumb2 when we are really using
>  > thumb2. Note that it is still necessary to pass this option, cfr.
>  > http://autobuild.buildroot.org/results/d7323831372050e425a34f5104a46d8cbd6be214
> 
>  > We could have chosen to still enable thumb2 with the internal toolchain.
>  > However, that would mean we create a discrepancy between the result of
>  > building with the same (buildroot) toolchain when it is used as an
>  > internal one or as an external one. To avoid potentially surprising
>  > results, just stick to the option chosen for the rest of the rootfs.
> 
> Why do we actually ever pass --enable-thumb2 / --disable-thumb2? As far
> as I can see, the only thing this does is adding -marm / -mthumb to
> CFLAGS, and we already pass the correct -m<arm/thumb> option in the
> toolchain wrapper.

 Well, if you don't pass anything, it will check if the _compiler_ supports
thumb by compiling (not linking) something which checks for __thumb2__. So we
have to pass something.

 Er, on second look, it uses that compile to set MOZ_THUMB2 and then doesn't do
anything with it... Still, there is a good chance that upstream will in the
future do something with that symbol, so I think it's safer to explicitly
en/disable thumb2.

 Regards,
 Arnout



-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] libnspr: only enable thumb if BR2_ARM_INSTRUCTIONS_THUMB2

[Arnout Vandecappelle]

libnspr currently passes --enable-thumb2 if the CPU has thumb
instructions. This option will pass -mthumb to the compiler. However,
if an external multilib toolchain is used that has a thumb-specific
variant (e.g. Sourcery), it will try to use that one. But we only copy
a single variant to the sysroot, so the build will fail with:

.../arm-none-linux-gnueabi/bin/ld: cannot find crti.o: No such file or directory
...
collect2: error: ld returned 1 exit status
../../config/rules.mk:303: recipe for target 'libnspr4.so' failed

In order to fix this, only enable thumb2 when we are really using
thumb2. Note that it is still necessary to pass this option, cfr.
http://autobuild.buildroot.org/results/d7323831372050e425a34f5104a46d8cbd6be214

We could have chosen to still enable thumb2 with the internal toolchain.
However, that would mean we create a discrepancy between the result of
building with the same (buildroot) toolchain when it is used as an
internal one or as an external one. To avoid potentially surprising
results, just stick to the option chosen for the rest of the rootfs.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Build-tested with various combinations of thumb, thumb2, floating point,
and external toolchains. I didn't test with an internal toolchain but
what could go wrong there? :-)

Changes v2: added missing build failure output to commit message
---
 package/libnspr/libnspr.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/libnspr/libnspr.mk b/package/libnspr/libnspr.mk
index 8e58986..c9bfa0c 100644
--- a/package/libnspr/libnspr.mk
+++ b/package/libnspr/libnspr.mk
@@ -50,7 +50,7 @@ LIBNSPR_INSTALL_STAGING_OPTS = DESTDIR=$(STAGING_DIR) LIBRARY= install
 endif
 
 ifeq ($(BR2_arm),y)
-ifeq ($(BR2_ARM_CPU_HAS_THUMB2),y)
+ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB2),y)
 LIBNSPR_CONF_OPTS += --enable-thumb2
 else
 LIBNSPR_CONF_OPTS += --disable-thumb2
-- 
2.7.0.rc3

[Buildroot] [PATCH] libnspr: only enable thumb if BR2_ARM_INSTRUCTIONS_THUMB2

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> writes:

 > libnspr currently passes --enable-thumb2 if the CPU has thumb
 > instructions. This option will pass -mthumb to the compiler. However,
 > if an external multilib toolchain is used that has a thumb-specific
 > variant (e.g. Sourcery), it will try to use that one. But we only copy
 > a single variant to the sysroot, so the build will fail with:

 > .../arm-none-linux-gnueabi/bin/ld: cannot find crti.o: No such file or directory
 > ...
 > collect2: error: ld returned 1 exit status
 > ../../config/rules.mk:303: recipe for target 'libnspr4.so' failed

 > In order to fix this, only enable thumb2 when we are really using
 > thumb2. Note that it is still necessary to pass this option, cfr.
 > http://autobuild.buildroot.org/results/d7323831372050e425a34f5104a46d8cbd6be214

 > We could have chosen to still enable thumb2 with the internal toolchain.
 > However, that would mean we create a discrepancy between the result of
 > building with the same (buildroot) toolchain when it is used as an
 > internal one or as an external one. To avoid potentially surprising
 > results, just stick to the option chosen for the rest of the rootfs.

 > Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
 > ---
 > Build-tested with various combinations of thumb, thumb2, floating point,
 > and external toolchains. I didn't test with an internal toolchain but
 > what could go wrong there? :-)

 > Changes v2: added missing build failure output to commit message
 > ---
 >  package/libnspr/libnspr.mk | 2 +-
 >  1 file changed, 1 insertion(+), 1 deletion(-)

 > diff --git a/package/libnspr/libnspr.mk b/package/libnspr/libnspr.mk
 > index 8e58986..c9bfa0c 100644
 > --- a/package/libnspr/libnspr.mk
 > +++ b/package/libnspr/libnspr.mk
 > @@ -50,7 +50,7 @@ LIBNSPR_INSTALL_STAGING_OPTS = DESTDIR=$(STAGING_DIR) LIBRARY= install
 >  endif
 
 >  ifeq ($(BR2_arm),y)
 > -ifeq ($(BR2_ARM_CPU_HAS_THUMB2),y)
 > +ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB2),y)
 >  LIBNSPR_CONF_OPTS += --enable-thumb2

Thanks. The conclusion was that --enable-thumb2 just caused it to pass
-mthumb in CFLAGS (and --disable-thumb2 to pass -marm), right?

What should we do for classic ARM cores in thumb1 mode? There we afaik
also want to use --enable-thumb2, but it could potentially break in
future if the option is changed to do something more than this.

-- 
Bye, Peter Korsgaard