code review 6946078: cmd/dist: make GOARM detection better compatible with t...

cmd/dist: make GOARM detection better compatible with thumb toolchain
Fixes issue 4557.

[dave_cheney.net, 2012-12-18 11:44:07]

Nice catch, I'm surprised (maybe I should not be) that GCC uses thumb by
default. 

Would a simpler solution be to change cmd/dist to enforce arm mode ?

Your solution looks fine, but I cannot comment on the implications of switching
to arm state from and unknown state, and if bx or will undo the mode switch.

[minux1, 2012-12-18 12:54:07]

PTAL. I think make.bash approach is better.

On Tue, Dec 18, 2012 at 7:44 PM, <dave@cheney.net> wrote:

> Nice catch, I'm surprised (maybe I should not be) that GCC uses thumb by
> default.
>
I should note that it's thumb2 and only for ARMv7-A/R.

modern gcc defaults to use thumb, esp. on armv7 where thumb2 provides
all the features of arm instruction set while reducing average instruction
set.

>
> Would a simpler solution be to change cmd/dist to enforce arm mode ?
>
we can change make.bash to always give -marm to gcc, and it will mirror
-m32 and -m64 for x86.

>
> Your solution looks fine, but I cannot comment on the implications of
> switching to arm state from and unknown state, and if bx or will undo
> the mode switch.
>
the instruction sequence i use can correctly switch from both arm and
thumb state to arm state and switch back (on ARMv5 or higher, or put it
another way, it will only fail on ARMv4, but I doubt people will compile Go
on that machine, because Go binary only supports ARMv5E).

[minux1, 2012-12-18 13:03:25]

On Tue, Dec 18, 2012 at 8:53 PM, minux <minux.ma@gmail.com> wrote:

> PTAL. I think make.bash approach is better.
>
scratch that. GOHOSTARCH is not set by default on ARM builds.

[minux1, 2012-12-18 15:20:06]

Hello dave@cheney.net (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[dave_cheney.net, 2012-12-18 21:14:11]

Fair enough, great job, I will test today. 

On 19/12/2012, at 0:03, minux <minux.ma@gmail.com> wrote:

> 
> On Tue, Dec 18, 2012 at 8:53 PM, minux <minux.ma@gmail.com> wrote:
>> PTAL. I think make.bash approach is better.
> scratch that. GOHOSTARCH is not set by default on ARM builds.

[dave_cheney.net, 2012-12-18 22:32:58]

On 2012/12/18 21:14:11, dfc wrote:
> Fair enough, great job, I will test today. 
> 
> On 19/12/2012, at 0:03, minux <mailto:minux.ma@gmail.com> wrote:
> 
> > 
> > On Tue, Dec 18, 2012 at 8:53 PM, minux <mailto:minux.ma@gmail.com> wrote:
> >> PTAL. I think make.bash approach is better.
> > scratch that. GOHOSTARCH is not set by default on ARM builds.

LGTM on ARMv7.

pandaboard(~) % echo $GOARM

pandaboard(~) % go tool dist env
GOROOT="/home/dfc/go"
GOBIN="/home/dfc/go/bin"
GOARCH="arm"
GOOS="linux"
GOHOSTARCH="arm"
GOHOSTOS="linux"
GOTOOLDIR="/home/dfc/go/pkg/tool/linux_arm"
GOCHAR="5"
GOARM="7"

Will test on the Pi this afternoon.

[dave_cheney.net, 2012-12-20 01:17:28]

LGTM on ARMv5 as well

[rsc, 2012-12-20 19:31:04]

LGTM

[minux1, 2012-12-21 18:40:14]

*** Submitted as https://code.google.com/p/go/source/detail?r=bff02defeb7a ***

cmd/dist: make GOARM detection better compatible with thumb toolchain
Fixes issue 4557.

R=dave, rsc
CC=golang-dev
https://codereview.appspot.com/6946078