[PATCH 0/2] Cortex M3 tune fixes

[PATCH 0/2] Cortex M3 tune fixes

[Andrei Gherzan]

Andrei Gherzan (2):
  feature-arm-thumb.inc: Use thumb instruction by default if
    TUNE_FEATURES thumb
  tune-cortexm3.inc: Cortex-M3 doesn't support ARM instructions

 meta/conf/machine/include/arm/feature-arm-thumb.inc | 2 +-
 meta/conf/machine/include/tune-cortexm3.inc         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--
1.8.1.4

[PATCH 1/2] feature-arm-thumb.inc: Use thumb instruction by default if TUNE_FEATURES thumb

[Andrei Gherzan]

Avoid a confusion when including thumb in TUNE_FATURES. In the curent behavior,
if I include thumb in TUNE_FATURES, ARM_THUMB_M_OPT will be "-marm" which makes
no sense for me as a default behavior. Obviously I can change this by using
ARM_INSTRUCTION_SET = thumb. But this seems strange and confusing. So let's do
it the other way around: have thumb instructions as default when TUNE_FATURES
contains "thumb" and switch to "arm" instructions when ARM_INSTRUCTION_SET is
"arm".

Signed-off-by: Andrei Gherzan <andrei@gherzan.ro>
---
 meta/conf/machine/include/arm/feature-arm-thumb.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/conf/machine/include/arm/feature-arm-thumb.inc b/meta/conf/machine/include/arm/feature-arm-thumb.inc
index bd754be..259f7fe 100644
--- a/meta/conf/machine/include/arm/feature-arm-thumb.inc
+++ b/meta/conf/machine/include/arm/feature-arm-thumb.inc
@@ -5,7 +5,7 @@
 # but requires more instructions (140% for 70% smaller code) so may be
 # slower.
 TUNEVALID[thumb] = "Use thumb instructions instead of ARM"
-ARM_THUMB_M_OPT = "${@['-marm', '-mthumb'][d.getVar('ARM_INSTRUCTION_SET', True) == 'thumb']}"
+ARM_THUMB_M_OPT = "${@['-mthumb', '-marm'][d.getVar('ARM_INSTRUCTION_SET', True) == 'arm']}"
 TUNE_CCARGS .= "${@bb.utils.contains("TUNE_FEATURES", "thumb", " ${ARM_THUMB_M_OPT}", "", d)}"
 OVERRIDES .= "${@bb.utils.contains("TUNE_FEATURES", "thumb", ":thumb", "", d)}"
 
-- 
1.8.1.4

Re: [PATCH 1/2] feature-arm-thumb.inc: Use thumb instruction by default if TUNE_FEATURES thumb

[Koen Kooi]

Op 11 jan. 2014, om 16:08 heeft Andrei Gherzan <andrei@gherzan.ro> het volgende geschreven:

> Avoid a confusion when including thumb in TUNE_FATURES. In the curent behavior,
> if I include thumb in TUNE_FATURES, ARM_THUMB_M_OPT will be "-marm" which makes
> no sense for me as a default behavior. Obviously I can change this by using
> ARM_INSTRUCTION_SET = thumb. But this seems strange and confusing. So let's do
> it the other way around: have thumb instructions as default when TUNE_FATURES
> contains "thumb" and switch to "arm" instructions when ARM_INSTRUCTION_SET is
> "arm".

Since ARM_INSTRUCTION_SET is unset for most things this will change the default, are you sure that this is what you want?

regards,

Koen

Re: [PATCH 1/2] feature-arm-thumb.inc: Use thumb instruction by default if TUNE_FEATURES thumb

[Andrei Gherzan]

[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]

On Sat, Jan 11, 2014 at 5:10 PM, Koen Kooi <koen@dominion.thruhere.net>wrote:

>
> Op 11 jan. 2014, om 16:08 heeft Andrei Gherzan <andrei@gherzan.ro> het
> volgende geschreven:
>
> > Avoid a confusion when including thumb in TUNE_FATURES. In the curent
> behavior,
> > if I include thumb in TUNE_FATURES, ARM_THUMB_M_OPT will be "-marm"
> which makes
> > no sense for me as a default behavior. Obviously I can change this by
> using
> > ARM_INSTRUCTION_SET = thumb. But this seems strange and confusing. So
> let's do
> > it the other way around: have thumb instructions as default when
> TUNE_FATURES
> > contains "thumb" and switch to "arm" instructions when
> ARM_INSTRUCTION_SET is
> > "arm".
>
> Since ARM_INSTRUCTION_SET is unset for most things this will change the
> default, are you sure that this is what you want?
>

I am aware of that but still, do you find it normal to add thumb as
TUNE_FEATURE and get your self with arm instructions flag?
And generally people adapt if changes make sense :)

ag

[-- Attachment #2: Type: text/html, Size: 1547 bytes --]

Re: [PATCH 1/2] feature-arm-thumb.inc: Use thumb instruction by default if TUNE_FEATURES thumb

[Koen Kooi]

Op 11 jan. 2014, om 16:16 heeft Andrei Gherzan <andrei@gherzan.ro> het volgende geschreven:

> 
> 
> 
> On Sat, Jan 11, 2014 at 5:10 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
> 
> Op 11 jan. 2014, om 16:08 heeft Andrei Gherzan <andrei@gherzan.ro> het volgende geschreven:
> 
> > Avoid a confusion when including thumb in TUNE_FATURES. In the curent behavior,
> > if I include thumb in TUNE_FATURES, ARM_THUMB_M_OPT will be "-marm" which makes
> > no sense for me as a default behavior. Obviously I can change this by using
> > ARM_INSTRUCTION_SET = thumb. But this seems strange and confusing. So let's do
> > it the other way around: have thumb instructions as default when TUNE_FATURES
> > contains "thumb" and switch to "arm" instructions when ARM_INSTRUCTION_SET is
> > "arm".
> 
> Since ARM_INSTRUCTION_SET is unset for most things this will change the default, are you sure that this is what you want?
> 
> I am aware of that but still, do you find it normal to add thumb as TUNE_FEATURE and get your self with arm instructions flag?

No, but I absolutely hate the ARM tune stuff in OE-core, you pretty much need anonymous python methods to change things on a distro level, which is crazy. It took me 2 weeks and a lot of help from Khem to switch angstrom to thumb2 mode on armv7a.

> And generally people adapt if changes make sense :)

What we actually need here is a way to say "this cpu can't to full ARM mode". Which is hard, since we are treating thumb as a superset instead of a subset. 

regards,

Koen

Re: [PATCH 1/2] feature-arm-thumb.inc: Use thumb instruction by default if TUNE_FEATURES thumb

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]

On Sat, Jan 11, 2014 at 05:16:27PM +0200, Andrei Gherzan wrote:
> On Sat, Jan 11, 2014 at 5:10 PM, Koen Kooi <koen@dominion.thruhere.net>wrote:
> 
> >
> > Op 11 jan. 2014, om 16:08 heeft Andrei Gherzan <andrei@gherzan.ro> het
> > volgende geschreven:
> >
> > > Avoid a confusion when including thumb in TUNE_FATURES. In the curent
> > behavior,
> > > if I include thumb in TUNE_FATURES, ARM_THUMB_M_OPT will be "-marm"
> > which makes
> > > no sense for me as a default behavior. Obviously I can change this by
> > using
> > > ARM_INSTRUCTION_SET = thumb. But this seems strange and confusing. So
> > let's do
> > > it the other way around: have thumb instructions as default when
> > TUNE_FATURES
> > > contains "thumb" and switch to "arm" instructions when
> > ARM_INSTRUCTION_SET is
> > > "arm".
> >
> > Since ARM_INSTRUCTION_SET is unset for most things this will change the
> > default, are you sure that this is what you want?
> >
> 
> I am aware of that but still, do you find it normal to add thumb as
> TUNE_FEATURE and get your self with arm instructions flag?
> And generally people adapt if changes make sense :)

With old behavior it was easier for DISTRO to have final say if it
supports thumb for MACHINEs with thumb in TUNE_FEATURES or not.

But with thumb-only MACHINE I see that this logic is going to be broken.

Should we introduce "arm" TUNE_FEATURE enabled in most MACHINEs (except
yours) and let feature-arm-thumb.inc respect that? (select "thumb" when
preferred or when there is no "arm" in TUNE_FEATUREs)?

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]

Re: [PATCH 1/2] feature-arm-thumb.inc: Use thumb instruction by default if TUNE_FEATURES thumb

[Kristof Robot]

On Sat, Jan 11, 2014 at 4:08 PM, Andrei Gherzan <andrei@gherzan.ro> wrote:
> Avoid a confusion when including thumb in TUNE_FATURES. In the curent behavior,
> if I include thumb in TUNE_FATURES, ARM_THUMB_M_OPT will be "-marm" which makes
> no sense for me as a default behavior.

I agree that this is confusing, I actually fell for this - thinking I
was using thumb instructions, while apparently I was not...

Any update on this patch?

Thanks,

Kristof

Re: [PATCH 1/2] feature-arm-thumb.inc: Use thumb instruction by default if TUNE_FEATURES thumb

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 798 bytes --]

On Mon, Feb 03, 2014 at 07:50:03PM +0100, Kristof Robot wrote:
> On Sat, Jan 11, 2014 at 4:08 PM, Andrei Gherzan <andrei@gherzan.ro> wrote:
> > Avoid a confusion when including thumb in TUNE_FATURES. In the curent behavior,
> > if I include thumb in TUNE_FATURES, ARM_THUMB_M_OPT will be "-marm" which makes
> > no sense for me as a default behavior.
> 
> I agree that this is confusing, I actually fell for this - thinking I
> was using thumb instructions, while apparently I was not...
> 
> Any update on this patch?

please test the patchset I've sent
http://lists.openembedded.org/pipermail/openembedded-core/2014-January/088665.html

it doesn't change the default, but it makes it easier to define thumb-only MACHINE
-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]

[PATCH 2/2] tune-cortexm3.inc: Cortex-M3 doesn't support ARM instructions

[Andrei Gherzan]

Use thumb instructions for Cortex-M3 to avoid this gcc error:
'error: target CPU does not support ARM mode'.

Extracted from "Cortex-M3 Technical Reference Manual":
"The processor implements the ARM v7-M architecture. This includes the entire
16-bit Thumb instruction set and the base Thumb-2 32-bit instruction set
architecture. The processor cannot execute ARM instructions."

Signed-off-by: Andrei Gherzan <andrei@gherzan.ro>
---
 meta/conf/machine/include/tune-cortexm3.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/conf/machine/include/tune-cortexm3.inc b/meta/conf/machine/include/tune-cortexm3.inc
index cbc4c0d..780baf4 100644
--- a/meta/conf/machine/include/tune-cortexm3.inc
+++ b/meta/conf/machine/include/tune-cortexm3.inc
@@ -10,5 +10,5 @@ TUNE_CCARGS .= "${@bb.utils.contains("TUNE_FEATURES", "armv7m", " -march=armv7-m
 
 AVAILTUNES += "cortexm3"
 ARMPKGARCH_tune-cortexm3 = "armv7m"
-TUNE_FEATURES_tune-cortexm3 = "armv7m vfp cortexm3"
+TUNE_FEATURES_tune-cortexm3 = "armv7m vfp cortexm3 thumb"
 PACKAGE_EXTRA_ARCHS_tune-cortexm3 = "${PACKAGE_EXTRA_ARCHS_tune-armv7at} armv7m-vfp"
-- 
1.8.1.4

Re: [PATCH 0/2] Cortex M3 tune fixes

[Trevor Woerner]

Since OE is generally used to build Linux distributions, I'm rather
curious to know why OE would include tunes for CortexM3?

Re: [PATCH 0/2] Cortex M3 tune fixes

[Phil Blundell]

On Mon, 2014-01-13 at 13:16 -0500, Trevor Woerner wrote:
> Since OE is generally used to build Linux distributions, I'm rather
> curious to know why OE would include tunes for CortexM3?

Well, Cortex-M can run uClinux.  And "generally" != "exclusively".

p.

Re: [PATCH 0/2] Cortex M3 tune fixes

[Trevor Woerner]

On 01/13/14 13:25, Phil Blundell wrote:
> On Mon, 2014-01-13 at 13:16 -0500, Trevor Woerner wrote:
>> Since OE is generally used to build Linux distributions, I'm rather
>> curious to know why OE would include tunes for CortexM3?
> Well, Cortex-M can run uClinux.

Is there an OE layer that adds support for building u-boot and uClinux
for Cortex-M3/4 [1]? I've been searching, and seem to be coming up empty.

If there isn't, then I'm back to my original question :-)



[1] https://github.com/EmcraftSystems

Re: [PATCH 0/2] Cortex M3 tune fixes

[Koen Kooi]

Op 13 jan. 2014, om 20:34 heeft Trevor Woerner <trevor.woerner@linaro.org> het volgende geschreven:

> On 01/13/14 13:25, Phil Blundell wrote:
>> On Mon, 2014-01-13 at 13:16 -0500, Trevor Woerner wrote:
>>> Since OE is generally used to build Linux distributions, I'm rather
>>> curious to know why OE would include tunes for CortexM3?
>> Well, Cortex-M can run uClinux.
> 
> Is there an OE layer that adds support for building u-boot and uClinux
> for Cortex-M3/4 [1]? I've been searching, and seem to be coming up empty.

You don't need a layer to build u-boot, it's in oe-core already.