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

Jump to bottom

math/bits: bits.Mul64 generates UMULH and MUL instruction on arm64 even if output is unused #54607

Closed
gbotrel opened this issue Aug 22, 2022 · 3 comments
Closed

math/bits: bits.Mul64 generates UMULH and MUL instruction on arm64 even if output is unused #54607

gbotrel opened this issue Aug 22, 2022 · 3 comments
Labels
FrozenDueToAge

Comments

@gbotrel
Copy link

gbotrel commented Aug 22, 2022

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

$ go version
go version go1.19 darwin/arm64

Does this issue reproduce with the latest release?

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

go env Output
$ go env
GOHOSTARCH="arm64"
GOHOSTOS="darwin"

What did you do?

func mulHI(v uint64) uint64 {
	hi, _ := bits.Mul64(v, v)
	return hi
}

func mulLO(v uint64) uint64 {
	_, lo := bits.Mul64(v, v) // lo = v * v work as expected
	return lo
}

What did you expect to see?

TEXT    main.mulHI(SB), LEAF|NOFRAME|ABIInternal, $0-8
UMULH   R0, R0, R1
// MUL     R0, R0, R2 <-- not needed
MOVD    R1, R0
RET     (R30)

TEXT    main.mulLO(SB), LEAF|NOFRAME|ABIInternal, $0-8
// UMULH   R0, R0, R1 <-- not needed
MUL     R0, R0, R2
MOVD    R2, R0
RET     (R30)

What did you see instead?

TEXT    main.mulHI(SB), LEAF|NOFRAME|ABIInternal, $0-8
UMULH   R0, R0, R1
MUL     R0, R0, R2
MOVD    R1, R0
RET     (R30)

TEXT    main.mulLO(SB), LEAF|NOFRAME|ABIInternal, $0-8
UMULH   R0, R0, R1
MUL     R0, R0, R2
MOVD    R2, R0
RET     (R30)
@gopherbot
Copy link

Change https://go.dev/cl/425101 mentions this issue: cmd/compile: split Muluhilo op on ARM64

@gopherbot
Copy link

Change https://go.dev/cl/425103 mentions this issue: cmd/compile: separate bits.Mul64 outputs on arm64

@gopherbot
Copy link

Change https://go.dev/cl/425203 mentions this issue: cmd/compile: deadcode for LoweredMuluhilo on riscv64

gopherbot pushed a commit that referenced this issue Aug 24, 2022
@wdvxdr1123 @gopherbot
cmd/compile: deadcode for LoweredMuluhilo on riscv64
b60432d 
This is a follow up of CL 425101 on RISCV64.

According to RISCV Volume 1, Unprivileged Spec v. 20191213 Chapter 7.1:
If both the high and low bits of the same product are required, then the
recommended code sequence is: MULH[[S]U] rdh, rs1, rs2; MUL rdl, rs1, rs2
(source register specifiers must be in same order and rdh cannot be the
same as rs1 or rs2). Microarchitectures can then fuse these into a single
multiply operation instead of performing two separate multiplies.

So we should not split Muluhilo to separate instructions.

Updates #54607

Change-Id: If47461f3aaaf00e27cd583a9990e144fb8bcdb17
Reviewed-on: https://go-review.googlesource.com/c/go/+/425203
Auto-Submit: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
rajbarik pushed a commit to rajbarik/go that referenced this issue Sep 1, 2022
@wdvxdr1123
cmd/compile: deadcode for LoweredMuluhilo on riscv64
53b6bbd 
This is a follow up of CL 425101 on RISCV64.

According to RISCV Volume 1, Unprivileged Spec v. 20191213 Chapter 7.1:
If both the high and low bits of the same product are required, then the
recommended code sequence is: MULH[[S]U] rdh, rs1, rs2; MUL rdl, rs1, rs2
(source register specifiers must be in same order and rdh cannot be the
same as rs1 or rs2). Microarchitectures can then fuse these into a single
multiply operation instead of performing two separate multiplies.

So we should not split Muluhilo to separate instructions.

Updates golang#54607

Change-Id: If47461f3aaaf00e27cd583a9990e144fb8bcdb17
Reviewed-on: https://go-review.googlesource.com/c/go/+/425203
Auto-Submit: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
@golang golang locked and limited conversation to collaborators Aug 24, 2023
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
@gopherbot @gbotrel