Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/asm: shift operands unsupported on arm64 (but supported on arm) #18070

Closed
williamweixiao opened this issue Nov 28, 2016 · 11 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@williamweixiao
Copy link
Member

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.7.3 linux/arm64

What operating system and processor architecture are you using (go env)?

GOARCH="arm64"
GOBIN=""
GOEXE=""
GOHOSTARCH="arm64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/weixia01/workspace/golang/gocode"
GORACE=""
GOROOT="/home/weixia01/workspace/go-go1.7.3"
GOTOOLDIR="/home/weixia01/workspace/go-go1.7.3/pkg/tool/linux_arm64"
CC="gcc"
GOGCCFLAGS="-fPIC -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build695199653=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

Assemble following assembly code (shiftopd.s):

1
2TEXT test?shiftopd(SB),0,$0-0
3 ADD R1>>1, R2, R0
4 RET

with command:
go tool asm shiftopd.s

What did you expect to see?

Assemble successfully

What did you see instead?

shiftopd.s:3: expected end of operand, found >>
asm: assembly of shiftopd.s failed

@williamweixiao williamweixiao changed the title cmd/asm:regression in support shift operands on arm64 cmd/asm:regression in shift operands on arm64 Nov 28, 2016
@davecheney
Copy link
Contributor

davecheney commented Nov 28, 2016 via email

@davecheney
Copy link
Contributor

davecheney commented Nov 28, 2016 via email

@gopherbot
Copy link

CL https://golang.org/cl/33595 mentions this issue.

@williamweixiao
Copy link
Member Author

a. according to go source code, shift operand is supported on arm but fail to work on armv8
b. i have added a test in: CL https://golang.org/cl/33595

@minux
Copy link
Member

minux commented Nov 28, 2016 via email

@williamweixiao
Copy link
Member Author

williamweixiao commented Nov 28, 2016

There is a discussion about the issue: https://groups.google.com/forum/#!msg/golang-nuts/yKmWV0WZZiw/n3JkTTQhBQAJ;context-place=forum/golang-nuts
Aram said

They used to be supported but I think I dropped them with the old C assembler.

If we just consider the issue since golang implementation switching from c to go, lack shift operand support on arm64 maybe a more suitable title

@mikioh mikioh changed the title cmd/asm:regression in shift operands on arm64 cmd/asm: regression in shift operands on arm64 Nov 28, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 29, 2016
@quentinmit quentinmit changed the title cmd/asm: regression in shift operands on arm64 cmd/asm: shift operands unsupported on arm64 (but supported on arm) Nov 29, 2016
@rsc
Copy link
Contributor

rsc commented Nov 29, 2016

I would like to wait until Go 1.9 for this, to give us more time to set up a proper test suite. This is the kind of thing that is easy to get wrong and hard to correct.

@rsc rsc added this to the Go1.9 milestone Nov 29, 2016
@rsc
Copy link
Contributor

rsc commented Nov 29, 2016

Ideally, the way this would proceed for Go 1.9 is the following. One thing that is missing from the arm64 port is a disassembler generated from a table of the instructions. See golang.org/x/arch's top-level directories for other examples. Once we have that, then we can generate a table of tests, commenting out the instructions that we don't have yet. Then when each new CL adds instructions it just uncomments the already-generated test, which gives us some confidence that the test is not just repeating what the CL implemented.

go/src/cmd/asm/internal/asm/testdata/amd64enc.s is an example of this.

@williamweixiao
Copy link
Member Author

williamweixiao commented Dec 6, 2016

We have started developing arm64 disassembler and expect to finish it within about three months.
As for using disassembler to generate table of tests, do you know where is the "x86test"?

@vielmetti
Copy link

Hi @williamweixiao can you give a progress report on the arm64 disassembler? Do you need any test resources? Let us know.

@williamweixiao
Copy link
Member Author

we have completed support for basic ARM64 instructions last month and are working on SIMD and FP instructions this month. we expect to make the first push to upstream within one month to support all instructions currently supported by assembler. Moreover, we need to discuss with upstream about how to express ARM64 SIMD instructions in go syntax which is still undefined in current assembler framework. I'm summarizing the materials and will post it ASAP.

lparth pushed a commit to lparth/go that referenced this issue Apr 13, 2017
Fixes: golang#18070
Also added a test in: cmd/asm/internal/asm/testdata/arm64.s

Change-Id: Icc43ff7383cc06b8eaccabd9ff0aefa61c4ecb88
Reviewed-on: https://go-review.googlesource.com/33595
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
@golang golang locked and limited conversation to collaborators Apr 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants