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

login
register
mail settings
Submitter Andrei Gherzan
Date Jan. 11, 2014, 3:08 p.m.
Message ID <1389452899-633-2-git-send-email-andrei@gherzan.ro>
Download mbox | patch
Permalink /patch/64571/
State New
Headers show

Comments

Andrei Gherzan - Jan. 11, 2014, 3:08 p.m.
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(-)
Koen Kooi - Jan. 11, 2014, 3:10 p.m.
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
Andrei Gherzan - Jan. 11, 2014, 3:16 p.m.
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
Koen Kooi - Jan. 11, 2014, 6:23 p.m.
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
Martin Jansa - Jan. 11, 2014, 11:19 p.m.
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)?
Kristof Robot - Feb. 3, 2014, 6:50 p.m.
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
Martin Jansa - Feb. 3, 2014, 7:06 p.m.
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

Patch

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)}"