feature-arm-thumb.inc: Do not tie generating thumb ISA to -march

Message ID 20190103071008.5065-1-raj.khem@gmail.com
State New
Headers show
Series
  • feature-arm-thumb.inc: Do not tie generating thumb ISA to -march
Related show

Commit Message

Khem Raj Jan. 3, 2019, 7:10 a.m.
-march=armv5't'e means that CPU can execute thumb ISA, we do not need to
tie this to exclusively generating thumb ISA, this change means that
when we have thumb in tune features then it can use 't' in -march
options irrespective of ISA being thumb or arm.

This fixes derivative of armv5 tunes and paves way for gcc9 where e.g.
armv5e is dropped and minimum arch supported is armv5te

Signed-off-by: Khem Raj <raj.khem@gmail.com>

---
 meta/conf/machine/include/arm/feature-arm-thumb.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.20.1

-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Comments

Martin Jansa Jan. 3, 2019, 7:27 a.m. | #1
I think this will include it not only in -march parameter but also in
TUNE_PKGARCH.
So the packages built with and without thumb ISA will have the same file
name and package feed which is wrong.

It should probably be fixed only when adding march to TUNE_CCARGS:
meta/conf/machine/include/arm/arch-armv4.inc:TUNE_CCARGS .=
"${@bb.utils.contains('TUNE_FEATURES', 'armv4', '
-march=armv4${ARMPKGSFX_THUMB}', '', d)}"
meta/conf/machine/include/arm/arch-armv5.inc:TUNE_CCARGS .=
"${@bb.utils.contains('TUNE_FEATURES', 'armv5', '
-march=armv5${ARMPKGSFX_THUMB}${ARMPKGSFX_DSP}', '', d)}"
meta/conf/machine/include/arm/arch-armv6.inc:TUNE_CCARGS .=
"${@bb.utils.contains('TUNE_FEATURES', 'armv6', ' -march=armv6', '', d)}"
meta/conf/machine/include/arm/arch-armv7a.inc:TUNE_CCARGS .=
"${@bb.utils.contains('TUNE_FEATURES', 'armv7a', ' -march=armv7-a', '', d)}"
meta/conf/machine/include/arm/arch-armv7ve.inc:TUNE_CCARGS .=
"${@bb.utils.contains('TUNE_FEATURES', 'armv7ve', ' -march=armv7ve', '',
d)}"


On Thu, Jan 3, 2019 at 8:10 AM Khem Raj <raj.khem@gmail.com> wrote:

> -march=armv5't'e means that CPU can execute thumb ISA, we do not need to

> tie this to exclusively generating thumb ISA, this change means that

> when we have thumb in tune features then it can use 't' in -march

> options irrespective of ISA being thumb or arm.

>

> This fixes derivative of armv5 tunes and paves way for gcc9 where e.g.

> armv5e is dropped and minimum arch supported is armv5te

>

> Signed-off-by: Khem Raj <raj.khem@gmail.com>

> ---

>  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 0b47ccad02..36bda6c67a 100644

> --- a/meta/conf/machine/include/arm/feature-arm-thumb.inc

> +++ b/meta/conf/machine/include/arm/feature-arm-thumb.inc

> @@ -24,7 +24,7 @@ python () {

>  TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb', '

> -m${ARM_M_OPT}', '', d)}"

>

>  # Add suffix from ARM_THUMB_SUFFIX only if after all this we still set

> ARM_M_OPT to thumb

> -ARMPKGSFX_THUMB .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb',

> '${ARM_THUMB_SUFFIX}', '', d) if d.getVar('ARM_M_OPT') == 'thumb' else ''}"

> +ARMPKGSFX_THUMB .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb',

> '${ARM_THUMB_SUFFIX}', '', d)}"

>

>  # what about armv7m devices which don't support -marm (e.g. Cortex-M3)?

>  TARGET_CC_KERNEL_ARCH += "${@bb.utils.contains('TUNE_FEATURES', 'thumb',

> '-mno-thumb-interwork -marm', '', d)}"

> --

> 2.20.1

>

> --

> _______________________________________________

> Openembedded-core mailing list

> Openembedded-core@lists.openembedded.org

> http://lists.openembedded.org/mailman/listinfo/openembedded-core

>
<div dir="ltr"><div dir="ltr"><div dir="ltr">I think this will include it not only in -march parameter but also in TUNE_PKGARCH.</div><div dir="ltr">So the packages built with and without thumb ISA will have the same file name and package feed which is wrong.</div><div dir="ltr"><br></div><div>It should probably be fixed only when adding march to TUNE_CCARGS:</div><div><div>meta/conf/machine/include/arm/arch-armv4.inc:TUNE_CCARGS .= &quot;${@bb.utils.contains(&#39;TUNE_FEATURES&#39;, &#39;armv4&#39;, &#39; -march=armv4${ARMPKGSFX_THUMB}&#39;, &#39;&#39;, d)}&quot;</div><div>meta/conf/machine/include/arm/arch-armv5.inc:TUNE_CCARGS .= &quot;${@bb.utils.contains(&#39;TUNE_FEATURES&#39;, &#39;armv5&#39;, &#39; -march=armv5${ARMPKGSFX_THUMB}${ARMPKGSFX_DSP}&#39;, &#39;&#39;, d)}&quot;</div><div>meta/conf/machine/include/arm/arch-armv6.inc:TUNE_CCARGS .= &quot;${@bb.utils.contains(&#39;TUNE_FEATURES&#39;, &#39;armv6&#39;, &#39; -march=armv6&#39;, &#39;&#39;, d)}&quot;</div><div>meta/conf/machine/include/arm/arch-armv7a.inc:TUNE_CCARGS .= &quot;${@bb.utils.contains(&#39;TUNE_FEATURES&#39;, &#39;armv7a&#39;, &#39; -march=armv7-a&#39;, &#39;&#39;, d)}&quot;</div><div>meta/conf/machine/include/arm/arch-armv7ve.inc:TUNE_CCARGS .= &quot;${@bb.utils.contains(&#39;TUNE_FEATURES&#39;, &#39;armv7ve&#39;, &#39; -march=armv7ve&#39;, &#39;&#39;, d)}&quot;</div></div><div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Jan 3, 2019 at 8:10 AM Khem Raj &lt;<a href="mailto:raj.khem@gmail.com">raj.khem@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">-march=armv5&#39;t&#39;e means that CPU can execute thumb ISA, we do not need to<br>
tie this to exclusively generating thumb ISA, this change means that<br>
when we have thumb in tune features then it can use &#39;t&#39; in -march<br>
options irrespective of ISA being thumb or arm.<br>
<br>
This fixes derivative of armv5 tunes and paves way for gcc9 where e.g.<br>
armv5e is dropped and minimum arch supported is armv5te<br>
<br>
Signed-off-by: Khem Raj &lt;<a href="mailto:raj.khem@gmail.com" target="_blank">raj.khem@gmail.com</a>&gt;<br>

---<br>
 meta/conf/machine/include/arm/feature-arm-thumb.inc | 2 +-<br>
 1 file changed, 1 insertion(+), 1 deletion(-)<br>
<br>
diff --git a/meta/conf/machine/include/arm/feature-arm-thumb.inc b/meta/conf/machine/include/arm/feature-arm-thumb.inc<br>
index 0b47ccad02..36bda6c67a 100644<br>
--- a/meta/conf/machine/include/arm/feature-arm-thumb.inc<br>
+++ b/meta/conf/machine/include/arm/feature-arm-thumb.inc<br>
@@ -24,7 +24,7 @@ python () {<br>
 TUNE_CCARGS .= &quot;${@bb.utils.contains(&#39;TUNE_FEATURES&#39;, &#39;thumb&#39;, &#39; -m${ARM_M_OPT}&#39;, &#39;&#39;, d)}&quot;<br>
<br>
 # Add suffix from ARM_THUMB_SUFFIX only if after all this we still set ARM_M_OPT to thumb<br>
-ARMPKGSFX_THUMB .= &quot;${@bb.utils.contains(&#39;TUNE_FEATURES&#39;, &#39;thumb&#39;, &#39;${ARM_THUMB_SUFFIX}&#39;, &#39;&#39;, d) if d.getVar(&#39;ARM_M_OPT&#39;) == &#39;thumb&#39; else &#39;&#39;}&quot;<br>
+ARMPKGSFX_THUMB .= &quot;${@bb.utils.contains(&#39;TUNE_FEATURES&#39;, &#39;thumb&#39;, &#39;${ARM_THUMB_SUFFIX}&#39;, &#39;&#39;, d)}&quot;<br>
<br>
 # what about armv7m devices which don&#39;t support -marm (e.g. Cortex-M3)?<br>
 TARGET_CC_KERNEL_ARCH += &quot;${@bb.utils.contains(&#39;TUNE_FEATURES&#39;, &#39;thumb&#39;, &#39;-mno-thumb-interwork -marm&#39;, &#39;&#39;, d)}&quot;<br>
-- <br>
2.20.1<br>
<br>
-- <br>
_______________________________________________<br>
Openembedded-core mailing list<br>
<a href="mailto:Openembedded-core@lists.openembedded.org" target="_blank">Openembedded-core@lists.openembedded.org</a><br>
<a href="http://lists.openembedded.org/mailman/listinfo/openembedded-core" rel="noreferrer" target="_blank">http://lists.openembedded.org/mailman/listinfo/openembedded-core</a><br>
</blockquote></div>
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Khem Raj Jan. 3, 2019, 7:34 a.m. | #2
On Wed, Jan 2, 2019 at 11:27 PM Martin Jansa <martin.jansa@gmail.com> wrote:
>

> I think this will include it not only in -march parameter but also in TUNE_PKGARCH.

> So the packages built with and without thumb ISA will have the same file name and package feed which is wrong.

>


I did ponder over it before following the approach I took, but I think
you have to point to maintain separate feeds for thumb and arm ISA
generated packages.
I think since we do not support OABI and EABI demands interwork, so
using 't' unconditionally in -march is a workable approach too.

> It should probably be fixed only when adding march to TUNE_CCARGS:

> meta/conf/machine/include/arm/arch-armv4.inc:TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv4', ' -march=armv4${ARMPKGSFX_THUMB}', '', d)}"

> meta/conf/machine/include/arm/arch-armv5.inc:TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv5', ' -march=armv5${ARMPKGSFX_THUMB}${ARMPKGSFX_DSP}', '', d)}"

> meta/conf/machine/include/arm/arch-armv6.inc:TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv6', ' -march=armv6', '', d)}"

> meta/conf/machine/include/arm/arch-armv7a.inc:TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv7a', ' -march=armv7-a', '', d)}"

> meta/conf/machine/include/arm/arch-armv7ve.inc:TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv7ve', ' -march=armv7ve', '', d)}"

>

>

> On Thu, Jan 3, 2019 at 8:10 AM Khem Raj <raj.khem@gmail.com> wrote:

>>

>> -march=armv5't'e means that CPU can execute thumb ISA, we do not need to

>> tie this to exclusively generating thumb ISA, this change means that

>> when we have thumb in tune features then it can use 't' in -march

>> options irrespective of ISA being thumb or arm.

>>

>> This fixes derivative of armv5 tunes and paves way for gcc9 where e.g.

>> armv5e is dropped and minimum arch supported is armv5te

>>

>> Signed-off-by: Khem Raj <raj.khem@gmail.com>

>> ---

>>  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 0b47ccad02..36bda6c67a 100644

>> --- a/meta/conf/machine/include/arm/feature-arm-thumb.inc

>> +++ b/meta/conf/machine/include/arm/feature-arm-thumb.inc

>> @@ -24,7 +24,7 @@ python () {

>>  TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb', ' -m${ARM_M_OPT}', '', d)}"

>>

>>  # Add suffix from ARM_THUMB_SUFFIX only if after all this we still set ARM_M_OPT to thumb

>> -ARMPKGSFX_THUMB .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb', '${ARM_THUMB_SUFFIX}', '', d) if d.getVar('ARM_M_OPT') == 'thumb' else ''}"

>> +ARMPKGSFX_THUMB .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb', '${ARM_THUMB_SUFFIX}', '', d)}"

>>

>>  # what about armv7m devices which don't support -marm (e.g. Cortex-M3)?

>>  TARGET_CC_KERNEL_ARCH += "${@bb.utils.contains('TUNE_FEATURES', 'thumb', '-mno-thumb-interwork -marm', '', d)}"

>> --

>> 2.20.1

>>

>> --

>> _______________________________________________

>> Openembedded-core mailing list

>> Openembedded-core@lists.openembedded.org

>> http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Khem Raj Jan. 3, 2019, 7:41 a.m. | #3
sent a v2 which i am testing

On Wed, Jan 2, 2019 at 11:34 PM Khem Raj <raj.khem@gmail.com> wrote:
>

> On Wed, Jan 2, 2019 at 11:27 PM Martin Jansa <martin.jansa@gmail.com> wrote:

> >

> > I think this will include it not only in -march parameter but also in TUNE_PKGARCH.

> > So the packages built with and without thumb ISA will have the same file name and package feed which is wrong.

> >

>

> I did ponder over it before following the approach I took, but I think

> you have to point to maintain separate feeds for thumb and arm ISA

> generated packages.

> I think since we do not support OABI and EABI demands interwork, so

> using 't' unconditionally in -march is a workable approach too.

>

> > It should probably be fixed only when adding march to TUNE_CCARGS:

> > meta/conf/machine/include/arm/arch-armv4.inc:TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv4', ' -march=armv4${ARMPKGSFX_THUMB}', '', d)}"

> > meta/conf/machine/include/arm/arch-armv5.inc:TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv5', ' -march=armv5${ARMPKGSFX_THUMB}${ARMPKGSFX_DSP}', '', d)}"

> > meta/conf/machine/include/arm/arch-armv6.inc:TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv6', ' -march=armv6', '', d)}"

> > meta/conf/machine/include/arm/arch-armv7a.inc:TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv7a', ' -march=armv7-a', '', d)}"

> > meta/conf/machine/include/arm/arch-armv7ve.inc:TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv7ve', ' -march=armv7ve', '', d)}"

> >

> >

> > On Thu, Jan 3, 2019 at 8:10 AM Khem Raj <raj.khem@gmail.com> wrote:

> >>

> >> -march=armv5't'e means that CPU can execute thumb ISA, we do not need to

> >> tie this to exclusively generating thumb ISA, this change means that

> >> when we have thumb in tune features then it can use 't' in -march

> >> options irrespective of ISA being thumb or arm.

> >>

> >> This fixes derivative of armv5 tunes and paves way for gcc9 where e.g.

> >> armv5e is dropped and minimum arch supported is armv5te

> >>

> >> Signed-off-by: Khem Raj <raj.khem@gmail.com>

> >> ---

> >>  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 0b47ccad02..36bda6c67a 100644

> >> --- a/meta/conf/machine/include/arm/feature-arm-thumb.inc

> >> +++ b/meta/conf/machine/include/arm/feature-arm-thumb.inc

> >> @@ -24,7 +24,7 @@ python () {

> >>  TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb', ' -m${ARM_M_OPT}', '', d)}"

> >>

> >>  # Add suffix from ARM_THUMB_SUFFIX only if after all this we still set ARM_M_OPT to thumb

> >> -ARMPKGSFX_THUMB .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb', '${ARM_THUMB_SUFFIX}', '', d) if d.getVar('ARM_M_OPT') == 'thumb' else ''}"

> >> +ARMPKGSFX_THUMB .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb', '${ARM_THUMB_SUFFIX}', '', d)}"

> >>

> >>  # what about armv7m devices which don't support -marm (e.g. Cortex-M3)?

> >>  TARGET_CC_KERNEL_ARCH += "${@bb.utils.contains('TUNE_FEATURES', 'thumb', '-mno-thumb-interwork -marm', '', d)}"

> >> --

> >> 2.20.1

> >>

> >> --

> >> _______________________________________________

> >> Openembedded-core mailing list

> >> Openembedded-core@lists.openembedded.org

> >> http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Patch

diff --git a/meta/conf/machine/include/arm/feature-arm-thumb.inc b/meta/conf/machine/include/arm/feature-arm-thumb.inc
index 0b47ccad02..36bda6c67a 100644
--- a/meta/conf/machine/include/arm/feature-arm-thumb.inc
+++ b/meta/conf/machine/include/arm/feature-arm-thumb.inc
@@ -24,7 +24,7 @@  python () {
 TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb', ' -m${ARM_M_OPT}', '', d)}"
 
 # Add suffix from ARM_THUMB_SUFFIX only if after all this we still set ARM_M_OPT to thumb
-ARMPKGSFX_THUMB .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb', '${ARM_THUMB_SUFFIX}', '', d) if d.getVar('ARM_M_OPT') == 'thumb' else ''}"
+ARMPKGSFX_THUMB .= "${@bb.utils.contains('TUNE_FEATURES', 'thumb', '${ARM_THUMB_SUFFIX}', '', d)}"
 
 # what about armv7m devices which don't support -marm (e.g. Cortex-M3)?
 TARGET_CC_KERNEL_ARCH += "${@bb.utils.contains('TUNE_FEATURES', 'thumb', '-mno-thumb-interwork -marm', '', d)}"