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: x86, incorrect Optab entry - PSRLW #13010

Closed
bjwbell opened this issue Oct 21, 2015 · 4 comments
Closed

cmd/asm: x86, incorrect Optab entry - PSRLW #13010

bjwbell opened this issue Oct 21, 2015 · 4 comments
Labels
FrozenDueToAge
Milestone
Go1.6

Comments

@bjwbell
Copy link

bjwbell commented Oct 21, 2015

In src/cmd/internal/obj/x86/asm6.go, the Optab table entry

{APSRLW, yps, Py3, [23]uint8{0xd1, 0x71, 02, Pe, 0xe1, Pe, 0x71, 02}},

should instead be

{APSRLW, yps, Py3, [23]uint8{0xd1, 0x71, 02, Pe, 0xd1, Pe, 0x71, 02}},

As it is now, if the shift amount is specified in a xmm register then PSRLW actually performs an arithmetic shift right i.e. PSRAW.

The issue can be reproduced with the below assembly function by calling it with shift := uint8(1) and x := [8]uint16{65535, 65535, 65535, 65535, 65535, 65535, 65535, 65535}

The expected return value is [8]uint16{32767, 32767, 32767, 32767, 32767, 32767, 32767, 32767}
The actual return value is [8]uint16{65535, 65535, 65535, 65535, 65535, 65535, 65535, 65535}

Apologies for the redundant loads/stores in the assembly, it's output from code generation.

// func shru16x8(x [8]uint16, shift uint8) [8]uint16 
TEXT ·shru16x8s(SB),NOSPLIT,$56-40
    MOVQ         $0, ret0+24(FP)
    MOVQ         $0, ret0+32(FP)
    MOVQ         $0, t0-16(SP)
    MOVQ         $0, t0-8(SP)
    MOVQ         $0, t2-48(SP)
    MOVQ         $0, t2-40(SP)
    MOVQ         $0, t1-32(SP)
    MOVQ         $0, t1-24(SP)
block0:
    MOVOU        x+0(FP), X15
    MOVOU        X15, t0-16(SP)
    MOVOU        t0-16(SP), X14
    MOVOU        X14, t1-32(SP)
    MOVB         shift+16(FP), R15
    MOVQ         R15, X15
    MOVOU        t1-32(SP), X14
    PSRLW        X15, X14
    MOVOU        X14, t2-48(SP)
    MOVOU        t2-48(SP), X15
    MOVOU        X15, ret0+24(FP)
    RET
@ianlancetaylor ianlancetaylor added this to the Go1.5.2 milestone Oct 21, 2015
@ianlancetaylor ianlancetaylor changed the title assembly: x86, incorrect Optab entry - PSRLW cmd/asm: x86, incorrect Optab entry - PSRLW Oct 21, 2015
@gopherbot
Copy link

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

@randall77
Copy link
Contributor

@bjwbell Thanks for the report.

bjwbell added a commit to bjwbell/gensimd that referenced this issue Oct 22, 2015
@bjwbell
Add SIMD u16x8 SHR support and SIMD u16x8 ref tests
5458e1c 
The u16x8 SHR support includes a workaround for a go compiler bug,
"cmd/asm: x86, incorrect Optab entry - PSRLW" golang/go#13010
@rsc
Copy link
Contributor

rsc commented Nov 13, 2015

This is bad but not bad enough to be in Go 1.5.2.

  • I confirmed that PSRLW is not generated by the compiler.
  • Go 1.5 and Go 1.5.1 (and probably earlier) will still have the problem.
  • Workaround for all of Go 1.5 is to use BYTE instructions.

@rsc rsc modified the milestones: Go1.6, Go1.5.2 Nov 13, 2015
@bjwbell
Copy link
Author

bjwbell commented Nov 16, 2015

Thanks Russ

bjwbell added a commit to bjwbell/gensimd that referenced this issue Jun 18, 2016
@bjwbell
Go 1.6 fixed golang/go#13010
0ba682f
@golang golang locked and limited conversation to collaborators Nov 16, 2016
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
Go1.6
Development

No branches or pull requests
5 participants
@bjwbell @rsc @ianlancetaylor @randall77 @gopherbot