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: unsupported instructions don't work on memory operands #21757

Closed
agnivade opened this issue Sep 4, 2017 · 1 comment
Closed

cmd/asm: unsupported instructions don't work on memory operands #21757

agnivade opened this issue Sep 4, 2017 · 1 comment
Labels
FrozenDueToAge

Comments

@agnivade
Copy link
Contributor

agnivade commented Sep 4, 2017

Please answer these questions before submitting your issue. Thanks!

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

master (1/2 days old). go version devel +60e743b Mon Sep 4 15:25:00 2017 +0530 linux/amd64

Does this issue reproduce with the latest release?

No. With Go 1.9, it works fine.

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/agniva/play/go"
GORACE=""
GOROOT="/home/agniva/play/gosource/go"
GOTOOLDIR="/home/agniva/play/gosource/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build456837155=/tmp/go-build -gno-record-gcc-switches"
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"

What did you do?

I was trying to optimize exp_amd64.s using VADDSD to combine a MOVSD and ADDSD instructions. But for that to work, the 3rd operand needs to be a memory location. When I was loading the memory to a register and then doing VADDSD it was working fine. But when I tried to access the memory location directly, it gave garbage.

Sample code -

Below is a code which tries to do VADDSD and VSUBSD. In first variation, they try to operate on registers, next, they work on memory operands.

// +build amd64
#include "textflag.h"

DATA mydata<>+0(SB)/8, $-8.75
GLOBL mydata<>+0(SB), RODATA, $8

// func vADDSDReg(x) float64
TEXT ·vADDSDReg(SB),NOSPLIT,$0
	MOVSD x+0(FP), X0
	MOVSD mydata<>+0(SB), X1
	// VADDSD X1, X0, X1 (dest op on left)
	BYTE $0xC5; BYTE $0xFB; BYTE $0x58; BYTE $0xC9
	MOVSD X1, ret+8(FP)
	RET

// func vADDSDMem(x) float64
TEXT ·vADDSDMem(SB),NOSPLIT,$0
	MOVSD x+0(FP), X0
	// VADDSD xmm1, xmm0, [rip+0x3feaa] (dest op on left)
	BYTE $0xC5; BYTE $0xFB; BYTE $0x58; BYTE $0x0D; BYTE $0xAA; BYTE $0xFE; BYTE $0x03; BYTE $0x00
	MOVSD X1, ret+8(FP)
	RET

// func vSUBSDReg(x) float64
TEXT ·vSUBSDReg(SB),NOSPLIT,$0
	MOVSD x+0(FP), X0
	MOVSD mydata<>+0(SB), X1
	// VSUBSD X1, X0, X1 (dest op on left)
	BYTE $0xC5; BYTE $0xFB; BYTE $0x5C; BYTE $0xC9
	MOVSD X1, ret+8(FP)
	RET

// func vSUBSDMem(x) float64
TEXT ·vSUBSDMem(SB),NOSPLIT,$0
	MOVSD x+0(FP), X0
	// VSUBSD xmm1, xmm0, [rip+0x3fe6a] (dest op on left)
	BYTE $0xC5; BYTE $0xFB; BYTE $0x5C; BYTE $0x0D; BYTE $0x6A; BYTE $0xFE; BYTE $0x03; BYTE $0x00
	MOVSD X1, ret+8(FP)
	RET

The rip hex offsets were extracted by doing a usual MOVSD mydata<>+0(SB), X1. And doing an objdump -d on the binary to get the actual instruction.

And the corresponding Go code which runs them-

package main

import (
	"fmt"
)

func vADDSDReg(x float64) float64

func vADDSDMem(x float64) float64

func vSUBSDReg(x float64) float64

func vSUBSDMem(x float64) float64

func main() {
	fmt.Println(vADDSDReg(4))
	fmt.Println(vADDSDMem(4))

	fmt.Println(vSUBSDReg(4))
	fmt.Println(vSUBSDMem(4))
}

What did you expect to see?

With Go 1.9

-4.75
-4.75
12.75
12.75

With Go master

-4.75
-4.75
12.75
12.75

What did you see instead?

With Go 1.9

-4.75
-4.75
12.75
12.75

With Go master

-4.75
-1.0564009196602605e+229
12.75
1.0564009196602605e+229

I haven't thoroughly checked every other command. But I am fairly confident that will happen on all unsupported instructions which try to use a memory operand.

Attached code for easy download and repro - bug.zip
@agnivade
Copy link
Contributor Author

Going to close this now since all AVX and AVX2 instructions are added to the assembler.

@golang golang locked and limited conversation to collaborators Dec 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@agnivade @gopherbot