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.

Sign up for GitHub

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

Jump to bottom

cmd/asm: illegal 4-operand instructions accepted on arm64 #25059

Closed
mengzhuo opened this issue Apr 25, 2018 · 7 comments
Closed

cmd/asm: illegal 4-operand instructions accepted on arm64 #25059

mengzhuo opened this issue Apr 25, 2018 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.11

Comments

@mengzhuo
Copy link
Contributor

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

master

go version devel +eca7a1343c Wed Apr 25 02:49:46 2018 +0000 linux/arm64
GOARCH="arm64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOHOSTARCH="arm64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/root/go"
GORACE=""
GOROOT="/root/godev"
GOTMPDIR=""
GOTOOLDIR="/root/godev/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-build121529311=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version devel +eca7a1343c Wed Apr 25 02:49:46 2018 +0000 linux/arm64
GOROOT/bin/go tool compile -V: compile version devel +eca7a1343c Wed Apr 25 02:49:46 2018 +0000
uname -sr: Linux 4.15-hikey
Distributor ID: Debian
Description:    Debian GNU/Linux 9.4 (stretch)
Release:        9.4
Codename:       stretch
/lib/aarch64-linux-gnu/libc.so.6: GNU C Library (Debian GLIBC 2.24-11+deb9u3) stable release version 2.24, by Roland McGrath et al.
gdb --version: GNU gdb (Debian 7.12-6) 7.12.0.20161007-git

What did you do?

Three MUL related instructions in t_arm64.s

        SMULL   R1, R2, R3, R4
        UMULL   R1, R2, R3, R4
        SMULH   R1, R2, R3, R4

What did you expect to see?

SMULL/UMULL as it should be
SMULH should failed.

What did you see instead?

9b210864                SMADDL R1, R2, R3, R4
9ba10864                UMADDL R1, R2, R3, R4
9b410864                SMULH R1, R3, R4
@mengzhuo
Copy link
Contributor Author

@williamweixiao @zhangfannie

@FiloSottile FiloSottile added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 25, 2018
@FiloSottile FiloSottile added this to the Go1.11 milestone Apr 25, 2018
@zhangfannie
Copy link
Contributor

The fix patch will be submit later.

@gopherbot
Copy link

Change https://golang.org/cl/109295 mentions this issue: cmd/asm: fix assemble MUL* related instructions on arm64 bug

@cherrymui
Copy link
Member

It also accepts ADD R1, R2, R3, R4 (assembles to ADD R1, R2, R4), and 4 operands for essentially every instruction.

The reason is that in the backend's optab we never check From3, except for the instructions which do have From3. For a complete fix I think we'd need to add From3 information to optab.

@cherrymui
Copy link
Member

And this probably applies to some of the other architectures as well.

@cherrymui cherrymui changed the title cmd/asm: wrong MUL* related instruction translate on arm64 cmd/asm: illegal 4-operand instructions accepted on arm64 Apr 25, 2018
@mengzhuo
Copy link
Contributor Author

But SMULL has compiled into SMADDL

@cherrymui
Copy link
Member

cherrymui commented Apr 25, 2018 •
edited

@mengzhuo, ok, we probably can fix that part first.

SMULL is an alias of SMADDL, with addend being the zero register. If we had check of From3 in optab, it would reject the 4-operand SMULL, and things would just work.

@golang golang locked and limited conversation to collaborators May 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
5 participants
@mengzhuo @FiloSottile @gopherbot @cherrymui @zhangfannie