On Fri, Mar 15, 2019 at 8:44 AM Mark Asselstine <mark.asselst...@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.asselst...@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.asselst...@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. -- _______________________________________________ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
