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/internal/obj/arm: wrong binary code generated for NMULA/NMULS #23212

Closed
benshi001 opened this issue Dec 21, 2017 · 2 comments
Closed

cmd/internal/obj/arm: wrong binary code generated for NMULA/NMULS #23212

benshi001 opened this issue Dec 21, 2017 · 2 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.10

Comments

@benshi001
Copy link
Member

benshi001 commented Dec 21, 2017 •
edited

In my previous commit 074547a

The NMULAF/NMULSF/NMULAD/NMULSD encoding are wrong. This is due to I was reading an old ARMv7 reference manual.

"NMULAF		F5, F6, F7" should be encoding to 457a16ee, but currently 057a16ee
"NMULAD		F5, F6, F7" should be encoding to 457b16ee, but currently 057b16ee
"NMULSF		F5, F6, F7" should be encoding to 057a16ee, but currently 457a16ee
"NMULSD		F5, F6, F7" should be encoding to 057b16ee, but currently 457b16ee

NMULA/NMULS are reversed due to a typo in older manual.

I have double checked with newer manual and gnu tool chain.

ee167a45 	vnmla.f32		s14, s12, s10
ee167b45 	vnmla.f64		d7, d6, d5
ee167a05 	vnmls.f32		s14, s12, s10
ee167b05 	vnmls.f64		d7, d6, d5
@gopherbot
Copy link

Change https://golang.org/cl/85116 mentions this issue: cmd/internal/obj/arm: fix wrong encoding of NMULA/NMULS

@ALTree ALTree added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 21, 2017
@ALTree
Copy link
Member

ALTree commented Dec 21, 2017 •
edited

Tentatively targeting to 1.10 since it fixes an issue that was introduced during the 1.10 cycle.

@ALTree ALTree added this to the Go1.10 milestone Dec 21, 2017
gopherbot pushed a commit that referenced this issue Dec 21, 2017
@benshi001 @cherrymui
cmd/internal/obj/arm: fix wrong encoding of NMULAF/NMULAD/NMULSF/NMULSD
14332ed 
NMULAF/NMULAD/NMULSF/NMULSD are incorrectly encoded by the arm
assembler.

Instruction            Right binary      Current wrong binary
"NMULAF	F5, F6, F7"    0xee167a45        0xee167a05
"NMULAD	F5, F6, F7"    0xee167b45        0xee167b05
"NMULSF	F5, F6, F7"    0xee167a05        0xee167a45
"NMULSD	F5, F6, F7"    0xee167b05        0xee167b45

This patch fixes this issue.

fixes issue #23212

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

No branches or pull requests
4 participants
@ALTree @gopherbot @cherrymui @benshi001