Patchwork [2/3] tune: remove thumb flag from non-thumb cortexa8 tune variables

login
register
mail settings
Submitter Darren Hart
Date Aug. 19, 2011, 10:06 p.m.
Message ID <fa0a22eee3461de38fc4b41a76f15980b78fd0bf.1313791350.git.dvhart@linux.intel.com>
Download mbox | patch
Permalink /patch/10307/
State New, archived
Headers show

Comments

Darren Hart - Aug. 19, 2011, 10:06 p.m.
The thumb flag "t" appears to have been copy/pasted to all the
PACKAGE_EXTRA_ARCHS tune variables. Remove it from the non-thumb versions.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Jason Kridner <jkridner@beagleboard.org>
CC: Koen Kooi <koen@dominion.thruhere.net>
---
 meta/conf/machine/include/tune-cortexa8.inc |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Khem Raj - Aug. 20, 2011, 4:01 p.m.
On 8/19/2011 3:06 PM, Darren Hart wrote:
> The thumb flag "t" appears to have been copy/pasted to all the
> PACKAGE_EXTRA_ARCHS tune variables. Remove it from the non-thumb versions.
>

hmmm non thumb versions can run thumb packages. So I think its ok to 
have t in EXTRA_ARCHS

> Signed-off-by: Darren Hart<dvhart@linux.intel.com>
> CC: Jason Kridner<jkridner@beagleboard.org>
> CC: Koen Kooi<koen@dominion.thruhere.net>
> ---
>   meta/conf/machine/include/tune-cortexa8.inc |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/meta/conf/machine/include/tune-cortexa8.inc b/meta/conf/machine/include/tune-cortexa8.inc
> index 02b560c..67c5f0b 100644
> --- a/meta/conf/machine/include/tune-cortexa8.inc
> +++ b/meta/conf/machine/include/tune-cortexa8.inc
> @@ -10,7 +10,7 @@ TUNE_FEATURES_tune-cortexa8 = "${TUNE_FEATURES_tune-armv7a} cortexa8"
>   TUNE_FEATURES_tune-cortexa8t = "${TUNE_FEATURES_tune-armv7at} cortexa8"
>   TUNE_FEATURES_tune-cortexa8-neon = "${TUNE_FEATURES_tune-cortexa8} neon"
>
> -PACKAGE_EXTRA_ARCHS_tune-cortexa8 = "${PACKAGE_EXTRA_ARCHS_tune-armv7at}"
> +PACKAGE_EXTRA_ARCHS_tune-cortexa8 = "${PACKAGE_EXTRA_ARCHS_tune-armv7a}"
>   PACKAGE_EXTRA_ARCHS_tune-cortexa8t = "${PACKAGE_EXTRA_ARCHS_tune-armv7at}"
> -PACKAGE_EXTRA_ARCHS_tune-cortexa8-neon = "${PACKAGE_EXTRA_ARCHS_tune-armv7at-neon}"
> +PACKAGE_EXTRA_ARCHS_tune-cortexa8-neon = "${PACKAGE_EXTRA_ARCHS_tune-armv7a-neon}"
>
Phil Blundell - Aug. 20, 2011, 5:58 p.m.
On Sat, 2011-08-20 at 09:01 -0700, Khem Raj wrote:
> On 8/19/2011 3:06 PM, Darren Hart wrote:
> > The thumb flag "t" appears to have been copy/pasted to all the
> > PACKAGE_EXTRA_ARCHS tune variables. Remove it from the non-thumb versions.
> >
> 
> hmmm non thumb versions can run thumb packages. So I think its ok to 
> have t in EXTRA_ARCHS

Er, really?  If it can run thumb code, in what sense is it a "non thumb"
version?

p.
Khem Raj - Aug. 20, 2011, 7:17 p.m.
On Sat, Aug 20, 2011 at 10:58 AM, Phil Blundell <philb@gnu.org> wrote:
> On Sat, 2011-08-20 at 09:01 -0700, Khem Raj wrote:
>> On 8/19/2011 3:06 PM, Darren Hart wrote:
>> > The thumb flag "t" appears to have been copy/pasted to all the
>> > PACKAGE_EXTRA_ARCHS tune variables. Remove it from the non-thumb versions.
>> >
>>
>> hmmm non thumb versions can run thumb packages. So I think its ok to
>> have t in EXTRA_ARCHS
>
> Er, really?  If it can run thumb code, in what sense is it a "non thumb"
> version?
>
It was for armv7 pov here not for all arms like v4
with EABI, on cortex and interworking being mandatory it should run thumb code
thats what I meant.
> p.
>
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
Darren Hart - Aug. 23, 2011, 6:07 p.m.
On 08/20/2011 09:01 AM, Khem Raj wrote:
> On 8/19/2011 3:06 PM, Darren Hart wrote:
>> The thumb flag "t" appears to have been copy/pasted to all the
>> PACKAGE_EXTRA_ARCHS tune variables. Remove it from the non-thumb versions.
>>
> 
> hmmm non thumb versions can run thumb packages. So I think its ok to 
> have t in EXTRA_ARCHS

Perhaps I'm confused with respect how these are used... see below.

> 
>> Signed-off-by: Darren Hart<dvhart@linux.intel.com>
>> CC: Jason Kridner<jkridner@beagleboard.org>
>> CC: Koen Kooi<koen@dominion.thruhere.net>
>> ---
>>   meta/conf/machine/include/tune-cortexa8.inc |    4 ++--
>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/meta/conf/machine/include/tune-cortexa8.inc b/meta/conf/machine/include/tune-cortexa8.inc
>> index 02b560c..67c5f0b 100644
>> --- a/meta/conf/machine/include/tune-cortexa8.inc
>> +++ b/meta/conf/machine/include/tune-cortexa8.inc
>> @@ -10,7 +10,7 @@ TUNE_FEATURES_tune-cortexa8 = "${TUNE_FEATURES_tune-armv7a} cortexa8"
>>   TUNE_FEATURES_tune-cortexa8t = "${TUNE_FEATURES_tune-armv7at} cortexa8"
>>   TUNE_FEATURES_tune-cortexa8-neon = "${TUNE_FEATURES_tune-cortexa8} neon"
>>
>> -PACKAGE_EXTRA_ARCHS_tune-cortexa8 = "${PACKAGE_EXTRA_ARCHS_tune-armv7at}"
>> +PACKAGE_EXTRA_ARCHS_tune-cortexa8 = "${PACKAGE_EXTRA_ARCHS_tune-armv7a}"

If we don't drop the t, do we need to add the non-t version as well?, ie:
-PACKAGE_EXTRA_ARCHS_tune-cortexa8 = "${PACKAGE_EXTRA_ARCHS_tune-armv7at}"
+PACKAGE_EXTRA_ARCHS_tune-cortexa8 = "${PACKAGE_EXTRA_ARCHS_tune-armv7a}
${PACKAGE_EXTRA_ARCHS_tune-armv7at}"



>>   PACKAGE_EXTRA_ARCHS_tune-cortexa8t = "${PACKAGE_EXTRA_ARCHS_tune-armv7at}"

Here we're adding thumb to thumb, so I took that to mean we needed to
add non-thumb to the non-thumb tune definitions.

>> -PACKAGE_EXTRA_ARCHS_tune-cortexa8-neon = "${PACKAGE_EXTRA_ARCHS_tune-armv7at-neon}"
>> +PACKAGE_EXTRA_ARCHS_tune-cortexa8-neon = "${PACKAGE_EXTRA_ARCHS_tune-armv7a-neon}"
Richard Purdie - Aug. 24, 2011, 1:25 a.m.
On Fri, 2011-08-19 at 15:06 -0700, Darren Hart wrote:
> The thumb flag "t" appears to have been copy/pasted to all the
> PACKAGE_EXTRA_ARCHS tune variables. Remove it from the non-thumb versions.
> 
> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
> CC: Jason Kridner <jkridner@beagleboard.org>
> CC: Koen Kooi <koen@dominion.thruhere.net>
> ---
>  meta/conf/machine/include/tune-cortexa8.inc |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/conf/machine/include/tune-cortexa8.inc b/meta/conf/machine/include/tune-cortexa8.inc
> index 02b560c..67c5f0b 100644
> --- a/meta/conf/machine/include/tune-cortexa8.inc
> +++ b/meta/conf/machine/include/tune-cortexa8.inc
> @@ -10,7 +10,7 @@ TUNE_FEATURES_tune-cortexa8 = "${TUNE_FEATURES_tune-armv7a} cortexa8"
>  TUNE_FEATURES_tune-cortexa8t = "${TUNE_FEATURES_tune-armv7at} cortexa8"
>  TUNE_FEATURES_tune-cortexa8-neon = "${TUNE_FEATURES_tune-cortexa8} neon"
>  
> -PACKAGE_EXTRA_ARCHS_tune-cortexa8 = "${PACKAGE_EXTRA_ARCHS_tune-armv7at}"
> +PACKAGE_EXTRA_ARCHS_tune-cortexa8 = "${PACKAGE_EXTRA_ARCHS_tune-armv7a}"
>  PACKAGE_EXTRA_ARCHS_tune-cortexa8t = "${PACKAGE_EXTRA_ARCHS_tune-armv7at}"
> -PACKAGE_EXTRA_ARCHS_tune-cortexa8-neon = "${PACKAGE_EXTRA_ARCHS_tune-armv7at-neon}"
> +PACKAGE_EXTRA_ARCHS_tune-cortexa8-neon = "${PACKAGE_EXTRA_ARCHS_tune-armv7a-neon}"

I'm not convinced this is a mistake. Even if you select the cortexa8
tune, the machine can still accept thumb packages, we're just not
building them so I think in these cases, PACKAGE_EXTRA_ARCHS is correct
and is intended to be more inclusive.

I've therefore not taken this change.

Cheers,

Richard

Patch

diff --git a/meta/conf/machine/include/tune-cortexa8.inc b/meta/conf/machine/include/tune-cortexa8.inc
index 02b560c..67c5f0b 100644
--- a/meta/conf/machine/include/tune-cortexa8.inc
+++ b/meta/conf/machine/include/tune-cortexa8.inc
@@ -10,7 +10,7 @@  TUNE_FEATURES_tune-cortexa8 = "${TUNE_FEATURES_tune-armv7a} cortexa8"
 TUNE_FEATURES_tune-cortexa8t = "${TUNE_FEATURES_tune-armv7at} cortexa8"
 TUNE_FEATURES_tune-cortexa8-neon = "${TUNE_FEATURES_tune-cortexa8} neon"
 
-PACKAGE_EXTRA_ARCHS_tune-cortexa8 = "${PACKAGE_EXTRA_ARCHS_tune-armv7at}"
+PACKAGE_EXTRA_ARCHS_tune-cortexa8 = "${PACKAGE_EXTRA_ARCHS_tune-armv7a}"
 PACKAGE_EXTRA_ARCHS_tune-cortexa8t = "${PACKAGE_EXTRA_ARCHS_tune-armv7at}"
-PACKAGE_EXTRA_ARCHS_tune-cortexa8-neon = "${PACKAGE_EXTRA_ARCHS_tune-armv7at-neon}"
+PACKAGE_EXTRA_ARCHS_tune-cortexa8-neon = "${PACKAGE_EXTRA_ARCHS_tune-armv7a-neon}"