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: wrong implement vmov/vld on arm64 #24400

Closed
mengzhuo opened this Issue Mar 15, 2018 · 9 comments

Comments

Projects
None yet
5 participants
@mengzhuo
Copy link
Contributor

mengzhuo commented Mar 15, 2018

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

go 1.10

Does this issue reproduce with the latest release?

yes

What did you do?

Try to compile this code

#include "textflag.h"

TEXT ·asm(SB),NOSPLIT,$0-0
    VMOV        R1, V30.S[1]
    VMOV        R1, V30.S[0]
    VLD1.P    8(R0), V2.D[1]
    VLD1      (R0), V2.B[14]

What did you expect to see?

All instructions compiled.

According to "as" these instructions should be (little endian

   1 0000 3E1C0C4E      mov    v30.s[1], w1
   2 0004 3E1C044E      mov    v30.s[0], w1
   3 0008 0284DF0D      ld1 {v2.d}[0], [x0], #8
   4 000c 0218404D      ld1 {v2.b}[14], [x0]

What did you see instead?

  t_arm64.s:4           0xe6d40                 4e041c3e                VMOV R1, V30.S[0]
  t_arm64.s:5           0xe6d44                 4e041c3e                VMOV R1, V30.S[0]

and

asm: illegal combination: 00008 (/root/go/src/t/t_arm64.s:6)    VLD1.P  8(R0), V2.D[1] PSOREG_8 NONE ELEM, 3 7
asm: illegal combination: 00012 (/root/go/src/t/t_arm64.s:7)    VLD1    (R0), V2.B[14] ZOREG NONE ELEM, 3 7

System details

go version go1.10 linux/arm64
GOARCH="arm64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOHOSTARCH="arm64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/root/go"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_arm64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build937154189=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.10 linux/arm64
GOROOT/bin/go tool compile -V: compile version go1.10
uname -sr: Linux 4.4.49-s5p6818
Distributor ID: Ubuntu
Description:    Ubuntu 16.04.4 LTS
Release:        16.04
Codename:       xenial
/lib/aarch64-linux-gnu/libc.so.6: GNU C Library (Ubuntu GLIBC 2.23-0ubuntu10) stable release version 2.23, by Roland McGrath et al.
gdb --version: GNU gdb (Ubuntu 7.11.1-0ubuntu1~16.5) 7.11.1
@mengzhuo

This comment has been minimized.

Copy link
Contributor Author

mengzhuo commented Mar 15, 2018

@cherrymui

This comment has been minimized.

Copy link
Contributor

cherrymui commented Mar 15, 2018

We don't support VLD1 load to a single element yet, only a list of arrangements (multiple structures) at the moment. Feel free to add the support :)

It seems the bug is that VMOV R1, V30.S[1] is assembled as VMOV R1, V30.S[0]?

cc @williamweixiao

@cherrymui cherrymui changed the title compile: wrong implement vmov/vld on arm64 cmd/asm: wrong implement vmov/vld on arm64 Mar 15, 2018

@mengzhuo

This comment has been minimized.

Copy link
Contributor Author

mengzhuo commented Mar 15, 2018

@cherrymui Well, that's beyond my skillsets.

This CL do has "vld into vector lane"
https://go-review.googlesource.com/c/go/+/77810
However it seems be forgotten.
Maybe we should split it into two CLs?

@cherrymui

This comment has been minimized.

Copy link
Contributor

cherrymui commented Mar 15, 2018

Yeah, splitting that CL to two CLs would be helpful. We can get the toolchain part in first.

@andybons andybons added the NeedsFix label Mar 15, 2018

@andybons andybons added this to the Go1.11 milestone Mar 15, 2018

@williamweixiao

This comment has been minimized.

Copy link
Member

williamweixiao commented Mar 16, 2018

It's a known bug to us. I'll let fangming split the CL to fix the toolchain part firstly.
Thanks for raising it!

@gopherbot

This comment has been minimized.

Copy link

gopherbot commented Mar 16, 2018

Change https://golang.org/cl/101095 mentions this issue: cmd/asm: add essential instructions for AES-GCM on ARM64

@gopherbot

This comment has been minimized.

Copy link

gopherbot commented Mar 19, 2018

Change https://golang.org/cl/101297 mentions this issue: cmd/asm: fix bug about VMOV instruction (move register to vector element) on ARM64

@gopherbot gopherbot closed this in 7673e30 Mar 20, 2018

@mengzhuo

This comment has been minimized.

Copy link
Contributor Author

mengzhuo commented Mar 20, 2018

Thanks for fixing VMOV bug.

However
https://go-review.googlesource.com/c/go/+/101095 not closed yet.

reopen this maybe?

@cherrymui

This comment has been minimized.

Copy link
Contributor

cherrymui commented Mar 20, 2018

Ok, reopened.

@cherrymui cherrymui reopened this Mar 20, 2018

@gopherbot gopherbot closed this in ef9bdd1 Apr 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.