* [v2][PATCH 1/2] go.bbclass: Export GOARM
@ 2019-03-14 20:04 Mark Asselstine
2019-03-14 20:04 ` [v2][PATCH 2/2] goarch.bbclass: set TARGET_GOARM as '7' for valid cortexa* targets Mark Asselstine
2019-03-15 4:39 ` [v2][PATCH 1/2] go.bbclass: Export GOARM Khem Raj
0 siblings, 2 replies; 8+ messages in thread
From: Mark Asselstine @ 2019-03-14 20:04 UTC (permalink / raw
To: openembedded-core, richard.purdie
When building GO packages for ARM (v5, v6, v7) it is expected that the
GOARM env variable is set during the build. Not having GOARM exported
will result in GO binaries which can't be executed on the target
(terminate with segfault). Per https://github.com/golang/go/wiki/GoArm
We already have the logic in goarch.bbclass to determine the correct
value of GOARM (see GOARM_TARGET), but currently this is only used to
build go itself (go-cross_1.*.bb requires go-cross.inc --> export
GOARM = "${TARGET_GOARM}").
Here we export GOARM but we do it such that it is only exported if we
are building for 'arm' and only when recipes requiring/inheriting
go.bbclass are be used to build for the target.
Signed-off-by: Mark Asselstine <mark.asselstine@windriver.com>
---
V2
* Avoid potential undefined situations
meta/classes/go.bbclass | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/meta/classes/go.bbclass b/meta/classes/go.bbclass
index af331f8..5af4c3e 100644
--- a/meta/classes/go.bbclass
+++ b/meta/classes/go.bbclass
@@ -8,6 +8,10 @@ GOROOT = "${STAGING_LIBDIR}/go"
export GOROOT
export GOROOT_FINAL = "${libdir}/go"
+GOARM[export] = "0"
+GOARM_arm_class-target = "${TARGET_GOARM}"
+GOARM_arm_class-target[export] = "1"
+
DEPENDS_GOLANG_class-target = "virtual/${TUNE_PKGARCH}-go virtual/${TARGET_PREFIX}go-runtime"
DEPENDS_GOLANG_class-native = "go-native"
DEPENDS_GOLANG_class-nativesdk = "virtual/${TARGET_PREFIX}go-crosssdk virtual/${TARGET_PREFIX}go-runtime"
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [v2][PATCH 2/2] goarch.bbclass: set TARGET_GOARM as '7' for valid cortexa* targets
2019-03-14 20:04 [v2][PATCH 1/2] go.bbclass: Export GOARM Mark Asselstine
@ 2019-03-14 20:04 ` Mark Asselstine
2019-03-14 23:44 ` Andre McCurdy
2019-03-15 4:39 ` [v2][PATCH 1/2] go.bbclass: Export GOARM Khem Raj
1 sibling, 1 reply; 8+ messages in thread
From: Mark Asselstine @ 2019-03-14 20:04 UTC (permalink / raw
To: openembedded-core, richard.purdie
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@windriver.com>
---
V2
* Cover all ARMv7 Cortex* variants
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [v2][PATCH 2/2] goarch.bbclass: set TARGET_GOARM as '7' for valid cortexa* targets
2019-03-14 20:04 ` [v2][PATCH 2/2] goarch.bbclass: set TARGET_GOARM as '7' for valid cortexa* targets Mark Asselstine
@ 2019-03-14 23:44 ` Andre McCurdy
2019-03-15 13:28 ` Mark Asselstine
0 siblings, 1 reply; 8+ messages in thread
From: Andre McCurdy @ 2019-03-14 23:44 UTC (permalink / raw
To: Mark Asselstine; +Cc: OE Core mailing list
On Thu, Mar 14, 2019 at 1:05 PM Mark Asselstine
<mark.asselstine@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@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? 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? Couldn't GOARM be set correctly just by using of
the _armv5, _armv6, _armv7a and _armv7ve over-rides?
https://github.com/golang/go/wiki/GoArm
> 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@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [v2][PATCH 1/2] go.bbclass: Export GOARM
2019-03-14 20:04 [v2][PATCH 1/2] go.bbclass: Export GOARM Mark Asselstine
2019-03-14 20:04 ` [v2][PATCH 2/2] goarch.bbclass: set TARGET_GOARM as '7' for valid cortexa* targets Mark Asselstine
@ 2019-03-15 4:39 ` Khem Raj
2019-03-15 12:48 ` Mark Asselstine
1 sibling, 1 reply; 8+ messages in thread
From: Khem Raj @ 2019-03-15 4:39 UTC (permalink / raw
To: Mark Asselstine; +Cc: Patches and discussions about the oe-core layer
On Thu, Mar 14, 2019 at 1:05 PM Mark Asselstine
<mark.asselstine@windriver.com> wrote:
>
> When building GO packages for ARM (v5, v6, v7) it is expected that the
> GOARM env variable is set during the build. Not having GOARM exported
> will result in GO binaries which can't be executed on the target
> (terminate with segfault). Per https://github.com/golang/go/wiki/GoArm
>
> We already have the logic in goarch.bbclass to determine the correct
> value of GOARM (see GOARM_TARGET), but currently this is only used to
> build go itself (go-cross_1.*.bb requires go-cross.inc --> export
> GOARM = "${TARGET_GOARM}").
>
> Here we export GOARM but we do it such that it is only exported if we
> are building for 'arm' and only when recipes requiring/inheriting
> go.bbclass are be used to build for the target.
>
> Signed-off-by: Mark Asselstine <mark.asselstine@windriver.com>
> ---
>
> V2
> * Avoid potential undefined situations
>
> meta/classes/go.bbclass | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/meta/classes/go.bbclass b/meta/classes/go.bbclass
> index af331f8..5af4c3e 100644
> --- a/meta/classes/go.bbclass
> +++ b/meta/classes/go.bbclass
> @@ -8,6 +8,10 @@ GOROOT = "${STAGING_LIBDIR}/go"
> export GOROOT
> export GOROOT_FINAL = "${libdir}/go"
>
> +GOARM[export] = "0"
> +GOARM_arm_class-target = "${TARGET_GOARM}"
> +GOARM_arm_class-target[export] = "1"
> +
while this works, I think it would be nicer to handle it for all
export GOHOSTOS = "${BUILD_GOOS}"
export GOHOSTARCH = "${BUILD_GOARCH}"
export GOOS = "${TARGET_GOOS}"
export GOARCH = "${TARGET_GOARCH}"
export GOARM = "${TARGET_GOARM}"
export GO386 = "${TARGET_GO386}"
export GOMIPS = "${TARGET_GOMIPS}"
these should all be replicated into bbclass.
> DEPENDS_GOLANG_class-target = "virtual/${TUNE_PKGARCH}-go virtual/${TARGET_PREFIX}go-runtime"
> DEPENDS_GOLANG_class-native = "go-native"
> DEPENDS_GOLANG_class-nativesdk = "virtual/${TARGET_PREFIX}go-crosssdk virtual/${TARGET_PREFIX}go-runtime"
> --
> 2.7.4
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [v2][PATCH 1/2] go.bbclass: Export GOARM
2019-03-15 4:39 ` [v2][PATCH 1/2] go.bbclass: Export GOARM Khem Raj
@ 2019-03-15 12:48 ` Mark Asselstine
0 siblings, 0 replies; 8+ messages in thread
From: Mark Asselstine @ 2019-03-15 12:48 UTC (permalink / raw
To: Khem Raj; +Cc: Patches and discussions about the oe-core layer
On Friday, March 15, 2019 12:39:26 AM EDT Khem Raj wrote:
> On Thu, Mar 14, 2019 at 1:05 PM Mark Asselstine
>
> <mark.asselstine@windriver.com> wrote:
> > When building GO packages for ARM (v5, v6, v7) it is expected that the
> > GOARM env variable is set during the build. Not having GOARM exported
> > will result in GO binaries which can't be executed on the target
> > (terminate with segfault). Per https://github.com/golang/go/wiki/GoArm
> >
> > We already have the logic in goarch.bbclass to determine the correct
> > value of GOARM (see GOARM_TARGET), but currently this is only used to
> > build go itself (go-cross_1.*.bb requires go-cross.inc --> export
> > GOARM = "${TARGET_GOARM}").
> >
> > Here we export GOARM but we do it such that it is only exported if we
> > are building for 'arm' and only when recipes requiring/inheriting
> > go.bbclass are be used to build for the target.
> >
> > Signed-off-by: Mark Asselstine <mark.asselstine@windriver.com>
> > ---
> >
> > V2
> > * Avoid potential undefined situations
> >
> > meta/classes/go.bbclass | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/meta/classes/go.bbclass b/meta/classes/go.bbclass
> > index af331f8..5af4c3e 100644
> > --- a/meta/classes/go.bbclass
> > +++ b/meta/classes/go.bbclass
> > @@ -8,6 +8,10 @@ GOROOT = "${STAGING_LIBDIR}/go"
> >
> > export GOROOT
> > export GOROOT_FINAL = "${libdir}/go"
> >
> > +GOARM[export] = "0"
> > +GOARM_arm_class-target = "${TARGET_GOARM}"
> > +GOARM_arm_class-target[export] = "1"
> > +
>
> while this works, I think it would be nicer to handle it for all
>
> export GOHOSTOS = "${BUILD_GOOS}"
> export GOHOSTARCH = "${BUILD_GOARCH}"
> export GOOS = "${TARGET_GOOS}"
> export GOARCH = "${TARGET_GOARCH}"
> export GOARM = "${TARGET_GOARM}"
> export GO386 = "${TARGET_GO386}"
> export GOMIPS = "${TARGET_GOMIPS}"
>
> these should all be replicated into bbclass.
I had thought about doing this but was using GOARM as a trial balloon as I had
a suitable build and testcase handy to sort through any issues. I will come
back around to these as I get time, if that's OK with folks.
MarkA
>
> > DEPENDS_GOLANG_class-target = "virtual/${TUNE_PKGARCH}-go
> > virtual/${TARGET_PREFIX}go-runtime" DEPENDS_GOLANG_class-native =
> > "go-native"
> > DEPENDS_GOLANG_class-nativesdk = "virtual/${TARGET_PREFIX}go-crosssdk
> > virtual/${TARGET_PREFIX}go-runtime">
> > --
> > 2.7.4
> >
> > --
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [v2][PATCH 2/2] goarch.bbclass: set TARGET_GOARM as '7' for valid cortexa* targets
2019-03-14 23:44 ` Andre McCurdy
@ 2019-03-15 13:28 ` Mark Asselstine
2019-03-15 15:44 ` Mark Asselstine
0 siblings, 1 reply; 8+ messages in thread
From: Mark Asselstine @ 2019-03-15 13:28 UTC (permalink / raw
To: Andre McCurdy; +Cc: OE Core mailing list
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@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@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@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [v2][PATCH 2/2] goarch.bbclass: set TARGET_GOARM as '7' for valid cortexa* targets
2019-03-15 13:28 ` Mark Asselstine
@ 2019-03-15 15:44 ` Mark Asselstine
2019-03-15 18:16 ` Andre McCurdy
0 siblings, 1 reply; 8+ messages in thread
From: Mark Asselstine @ 2019-03-15 15:44 UTC (permalink / raw
To: Andre McCurdy; +Cc: OE Core mailing list
On Friday, March 15, 2019 9:28:16 AM EDT Mark Asselstine wrote:
> 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@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@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.
The MACHINEOVERRIDES do already distill down the TUNE_FEATURES so yes we could
potentially use this to the logic from getting more complex. After digging
around the various tuning/maching/arch files I don't see any exceptions (at
least none are immediately visible). Would you be happier with the following:
---
HOST_GOARCH = "${@go_map_arch(d.getVar('HOST_ARCH'), d)}"
-HOST_GOARM = "${@go_map_arm(d.getVar('HOST_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
+HOST_GOARM = "${@go_map_arm(d.getVar('HOST_ARCH'), d.getVar('BASE_GOARM'), d)}"
HOST_GO386 = "${@go_map_386(d.getVar('HOST_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
HOST_GOMIPS = "${@go_map_mips(d.getVar('HOST_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
HOST_GOTUPLE = "${HOST_GOOS}_${HOST_GOARCH}"
TARGET_GOOS = "${@go_map_os(d.getVar('TARGET_OS'), d)}"
TARGET_GOARCH = "${@go_map_arch(d.getVar('TARGET_ARCH'), d)}"
-TARGET_GOARM = "${@go_map_arm(d.getVar('TARGET_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
+TARGET_GOARM = "${@go_map_arm(d.getVar('TARGET_ARCH'), d.getVar('BASE_GOARM'), d)}"
TARGET_GO386 = "${@go_map_386(d.getVar('TARGET_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
TARGET_GOMIPS = "${@go_map_mips(d.getVar('TARGET_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
TARGET_GOTUPLE = "${TARGET_GOOS}_${TARGET_GOARCH}"
GO_BUILD_BINDIR = "${@['bin/${HOST_GOTUPLE}','bin'][d.getVar('BUILD_GOTUPLE') == d.getVar('HOST_GOTUPLE')]}"
+# Use the MACHINEOVERRIDES to map ARM CPU architecture passed to GO via GOARM.
+# This is combined with *_ARCH to set HOST_GOARM and TARGET_GOARM.
+BASE_GOARM = ''
+BASE_GOARM_armv7ve = '7'
+BASE_GOARM_armv7a = '7'
+BASE_GOARM_armv6 = '6'
+BASE_GOARM_armv5 = '5'
+
# Go supports dynamic linking on a limited set of architectures.
# See the supportsDynlink function in go/src/cmd/compile/internal/gc/main.go
GO_DYNLINK = ""
@@ -74,12 +82,7 @@ def go_map_arch(a, d):
def go_map_arm(a, f, d):
import re
if re.match('arm.*', a):
- if 'armv7' in f:
- return '7'
- elif 'armv6' in f:
- return '6'
- elif 'armv5' in f:
- return '5'
+ return f
return ''
---
Before I send out a V3 can you comment if the above would be
inline with your expectations?
Thanks
MarkA
>
> > 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@lists.openembedded.org
> > > http://lists.openembedded.org/mailman/listinfo/openembedded-core
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [v2][PATCH 2/2] goarch.bbclass: set TARGET_GOARM as '7' for valid cortexa* targets
2019-03-15 15:44 ` Mark Asselstine
@ 2019-03-15 18:16 ` Andre McCurdy
0 siblings, 0 replies; 8+ messages in thread
From: Andre McCurdy @ 2019-03-15 18:16 UTC (permalink / raw
To: Mark Asselstine; +Cc: OE Core mailing list
On Fri, Mar 15, 2019 at 8:44 AM Mark Asselstine
<mark.asselstine@windriver.com> wrote:
>
> On Friday, March 15, 2019 9:28:16 AM EDT Mark Asselstine wrote:
> > 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@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@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.
>
> The MACHINEOVERRIDES do already distill down the TUNE_FEATURES so yes we could
> potentially use this to the logic from getting more complex. After digging
> around the various tuning/maching/arch files I don't see any exceptions (at
> least none are immediately visible). Would you be happier with the following:
>
> ---
> HOST_GOARCH = "${@go_map_arch(d.getVar('HOST_ARCH'), d)}"
> -HOST_GOARM = "${@go_map_arm(d.getVar('HOST_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
> +HOST_GOARM = "${@go_map_arm(d.getVar('HOST_ARCH'), d.getVar('BASE_GOARM'), d)}"
> HOST_GO386 = "${@go_map_386(d.getVar('HOST_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
> HOST_GOMIPS = "${@go_map_mips(d.getVar('HOST_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
> HOST_GOTUPLE = "${HOST_GOOS}_${HOST_GOARCH}"
> TARGET_GOOS = "${@go_map_os(d.getVar('TARGET_OS'), d)}"
> TARGET_GOARCH = "${@go_map_arch(d.getVar('TARGET_ARCH'), d)}"
> -TARGET_GOARM = "${@go_map_arm(d.getVar('TARGET_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
> +TARGET_GOARM = "${@go_map_arm(d.getVar('TARGET_ARCH'), d.getVar('BASE_GOARM'), d)}"
> TARGET_GO386 = "${@go_map_386(d.getVar('TARGET_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
> TARGET_GOMIPS = "${@go_map_mips(d.getVar('TARGET_ARCH'), d.getVar('TUNE_FEATURES'), d)}"
> TARGET_GOTUPLE = "${TARGET_GOOS}_${TARGET_GOARCH}"
> GO_BUILD_BINDIR = "${@['bin/${HOST_GOTUPLE}','bin'][d.getVar('BUILD_GOTUPLE') == d.getVar('HOST_GOTUPLE')]}"
>
> +# Use the MACHINEOVERRIDES to map ARM CPU architecture passed to GO via GOARM.
> +# This is combined with *_ARCH to set HOST_GOARM and TARGET_GOARM.
> +BASE_GOARM = ''
> +BASE_GOARM_armv7ve = '7'
> +BASE_GOARM_armv7a = '7'
> +BASE_GOARM_armv6 = '6'
> +BASE_GOARM_armv5 = '5'
> +
> # Go supports dynamic linking on a limited set of architectures.
> # See the supportsDynlink function in go/src/cmd/compile/internal/gc/main.go
> GO_DYNLINK = ""
> @@ -74,12 +82,7 @@ def go_map_arch(a, d):
> def go_map_arm(a, f, d):
> import re
> if re.match('arm.*', a):
> - if 'armv7' in f:
> - return '7'
> - elif 'armv6' in f:
> - return '6'
> - elif 'armv5' in f:
> - return '5'
> + return f
> return ''
> ---
>
> Before I send out a V3 can you comment if the above would be
> inline with your expectations?
This looks OK to me.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-15 18:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-14 20:04 [v2][PATCH 1/2] go.bbclass: Export GOARM Mark Asselstine
2019-03-14 20:04 ` [v2][PATCH 2/2] goarch.bbclass: set TARGET_GOARM as '7' for valid cortexa* targets Mark Asselstine
2019-03-14 23:44 ` Andre McCurdy
2019-03-15 13:28 ` Mark Asselstine
2019-03-15 15:44 ` Mark Asselstine
2019-03-15 18:16 ` Andre McCurdy
2019-03-15 4:39 ` [v2][PATCH 1/2] go.bbclass: Export GOARM Khem Raj
2019-03-15 12:48 ` Mark Asselstine
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.