[OE-core] [v2][PATCH 2/2] goarch.bbclass: set TARGET_GOARM as '7' for valid cortexa* targets

[Mark Asselstine]

On Thursday, March 14, 2019 7:44:36 PM EDT Andre McCurdy wrote:
> On Thu, Mar 14, 2019 at 1:05 PM Mark Asselstine
> 
> <mark.asselstine at windriver.com> wrote:
> > Per https://github.com/golang/go/wiki/GoArm we need to set GOARM when
> > building for ARMv5, ARMv6 and ARMv7. The current code in go_map_arm()
> > does not account for the Cortex* TUNINGs and as such misses several
> > Cortex variants which implement ARMv7. Here we add an additional check
> > that will include Cortex-A5 through Cortex-A17 as ARMv7 variants
> > (ie. GOARM='7'). The Cortex-R and Cortex-M variants are also captured
> > even though there are no tunings for these currently (lack MMU support
> > so not supported).
> > 
> > Signed-off-by: Mark Asselstine <mark.asselstine at windriver.com>
> > ---
> > 
> > V2
> > * Cover all ARMv7 Cortex* variants
> 
> Still worried this might not be complete. It's basically fixing the
> fallout from Khem's patches to remove -march options from the CPU
> specific ARM machine includes, right? 

Although this came in via meta-virtualization, which I usually review most 
changes, I wasn't involved in the original implementation or discussion of 
go_map_arm() and since I was simply extending the existing framework I didn't 
go back and look into the history.

Looking now I don't see any specific discussions as to why TUNE_FEATURES were 
used instead of something else, such as the overrides. My guess is it had to 
be something and the original implementation was incorrectly oversimplified so 
the level of complexity was equivalent to any other choice.

At any rate I will review if changing to something else such as overrides 
would simplify the now more complex logic but until I investigate I would 
hesitate to say that the approach will be any less prone to breaking.

> If so then any ARM machine with
> CPU specific tuning (other than cortex) is still going to be affected.
> 
> What's the reason for parsing TUNE_FEATURES to detect the ARM
> architecture level?

That's just what was in place when I started to work on this.

> Couldn't GOARM be set correctly just by using of
> the _armv5, _armv6, _armv7a and _armv7ve over-rides?

At a quick glance I would say yes, but again I need to poke around to see if 
this is just swapping one thing for another with no real gains.

> 
>   https://github.com/golang/go/wiki/GoArm

I already reference this in my commit log. Is there a reason you are 
highlighting this again?

MarkA

> 
> >  meta/classes/goarch.bbclass | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/meta/classes/goarch.bbclass b/meta/classes/goarch.bbclass
> > index 39fea5e..84977a5 100644
> > --- a/meta/classes/goarch.bbclass
> > +++ b/meta/classes/goarch.bbclass
> > 
> > @@ -74,6 +74,10 @@ def go_map_arch(a, d):
> >  def go_map_arm(a, f, d):
> >      import re
> > 
> >      if re.match('arm.*', a):
> > +        for el in f.split():
> > +            m = re.match("cortex[arm](\d.*)", el)
> > +            if m and int(m.group(1)) < 32:
> > +               return '7'
> > 
> >          if 'armv7' in f:
> >              return '7'
> >          
> >          elif 'armv6' in f:
> > --
> > 2.7.4
> > 
> > --
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core at lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core