Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1725)

Issue 5987063: code review 5987063: cmd/5l: use GOARM=7 to gate the VFPv3-only "vmov (imm)"... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 8 months ago by minux1
Modified:
12 years, 2 months ago
Reviewers:
rsc, dave
CC:
golang-dev
Visibility:
Public.

Description

cmd/5l: use GOARM=7 to gate the VFPv3-only "vmov (imm)" instruction Fixes issue 3456.

Patch Set 1 #

Patch Set 2 : diff -r 734071724054 https://code.google.com/p/go/ #

Patch Set 3 : diff -r 734071724054 https://code.google.com/p/go/ #

Patch Set 4 : diff -r 734071724054 https://code.google.com/p/go/ #

Patch Set 5 : diff -r 316890203045 https://code.google.com/p/go #

Patch Set 6 : diff -r d8e47164f8dd https://code.google.com/p/go/ #

Patch Set 7 : diff -r 903a3cdd92cf https://code.google.com/p/go/ #

Patch Set 8 : diff -r 6e5762cf9a29 https://code.google.com/p/go/ #

Patch Set 9 : diff -r 427e6db53ab5 https://code.google.com/p/go/ #

Patch Set 10 : diff -r 7f6a0510d3c9 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -3 lines) Patch
M src/cmd/5l/asm.c View 1 2 3 4 5 6 2 chunks +8 lines, -1 line 0 comments Download
M src/cmd/5l/l.h View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M src/cmd/5l/obj.c View 1 2 3 4 1 chunk +9 lines, -2 lines 0 comments Download

Messages

Total messages: 5
minux1
Hello dave@cheney.net (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
12 years, 5 months ago (2012-06-13 15:20:02 UTC) #1
rsc
I would rather stop using the instruction altogether than introduce a new GOARM setting. Russ
12 years, 5 months ago (2012-06-13 15:33:56 UTC) #2
dave_cheney.net
On a pandaboard (ARMv7) the performance hit for applying this patch is quite high for ...
12 years, 4 months ago (2012-07-10 21:54:49 UTC) #3
rsc
I'd be a bit less uneasy about this if we could make the binaries detect ...
12 years, 4 months ago (2012-08-05 23:00:45 UTC) #4
minux1
12 years, 4 months ago (2012-08-06 02:29:44 UTC) #5
On Mon, Aug 6, 2012 at 7:00 AM, <rsc@golang.org> wrote:

> I'd be a bit less uneasy about this if we could make the binaries detect
> when they are being used on systems that are too old.
>
this detection would be easy given that we already parse runtime·hwcap from
auxv.

or, better yet, we detect this from sigill handler, and this would be more
reliable
and portable (by portable, i mean portable to my Darwin/ARM port, although
if
you think judging from runtime·hwcap is ok, i will be fine, because
Darwin/ARM
that is capable of running Go would be always Cortex-A8 or higher so this
is a
non issue). the downside of this is that maybe we need to parse the
instruction
to see if it is vmov imm, or some other floating point instructions; but
the main
benefit would be, if the program doesn't use the offending instructions, we
don't
need to tell the user to change anything (this is not the case for our
current runtime,
because it will always use floating point instruction in runtime.check(),
but at least
we can run successfully on VFPv2 systems). of course, from another
perspective,
you might think this kind of uncertainty is the worst feature.

which solution do you think is better?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b