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

cmd/compile: non-constant rotations are not emitted #18940

Closed
rasky opened this issue Feb 4, 2017 · 4 comments
Closed

cmd/compile: non-constant rotations are not emitted #18940

rasky opened this issue Feb 4, 2017 · 4 comments
Labels
FrozenDueToAge Performance
Milestone
Go1.9Maybe

Comments

@rasky
Copy link
Member

rasky commented Feb 4, 2017

Please answer these questions before submitting your issue. Thanks!

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

`go version devel +34b455da44 Sat Feb 4 07:24:20 2017 +0000 darwin/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/rasky/Sources/go"
GORACE=""
GOROOT="/Users/rasky/Sources/gosrc"
GOTOOLDIR="/Users/rasky/Sources/gosrc/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/lw/jdbk7p_d4gj6qpydczpbw2080000gn/T/go-build646363960=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

func testRotate_easy(op uint32, cnt uint) uint32 {
	cnt = cnt & 31
	return (op << cnt) | (op << (32 - cnt))
}

func testRotate_complex(op uint32) uint32 {
	rot := uint((op >> 7) & 0x1E)
	return ((op & 0xFF) >> rot) | ((op & 0xFF) << (32 - rot))
}

What did you expect to see?

Rotate opcode emitted

What did you see instead?

Rotate opcode not emitted
@rasky
Copy link
Member Author

rasky commented Feb 4, 2017

/cc @randall77

@randall77
Copy link
Contributor

See also #18616 (addbits.RotateLeft).

@rasky rasky mentioned this issue Feb 6, 2017
Closed
@bradfitz bradfitz added this to the Go1.9Maybe milestone Mar 21, 2017
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Apr 17, 2017
@randall77
cmd/compile: implement non-constant rotates
7e07e63 
Makes math/bits.Rotate{Left,Right} fast on amd64.

name              old time/op  new time/op  delta
RotateLeft-12     7.42ns ± 6%  5.45ns ± 6%  -26.54%   (p=0.000 n=9+10)
RotateLeft8-12    4.77ns ± 5%  3.42ns ± 7%  -28.25%   (p=0.000 n=8+10)
RotateLeft16-12   4.82ns ± 8%  3.40ns ± 7%  -29.36%  (p=0.000 n=10+10)
RotateLeft32-12   4.87ns ± 7%  3.48ns ± 7%  -28.51%    (p=0.000 n=8+9)
RotateLeft64-12   5.23ns ±10%  3.35ns ± 6%  -35.97%   (p=0.000 n=9+10)
RotateRight-12    7.59ns ± 8%  5.71ns ± 1%  -24.72%   (p=0.000 n=10+8)
RotateRight8-12   4.98ns ± 7%  3.36ns ± 9%  -32.55%  (p=0.000 n=10+10)
RotateRight16-12  5.12ns ± 2%  3.45ns ± 5%  -32.62%  (p=0.000 n=10+10)
RotateRight32-12  4.80ns ± 6%  3.42ns ±16%  -28.68%  (p=0.000 n=10+10)
RotateRight64-12  4.78ns ± 6%  3.42ns ± 6%  -28.50%  (p=0.000 n=10+10)

Update #18940

Change-Id: Ie79fb5581c489ed4d3b859314c5e669a134c119b
Reviewed-on: https://go-review.googlesource.com/39711
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
@randall77
Copy link
Contributor

I'm going to close this as done. The easy case is handled now as is, or using bits.RotateLeft.
The hard case seems less pressing, and you can get the rotate behavior by either adding an additional &0x1f or using bits.Rotateleft.

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

No branches or pull requests
4 participants
@bradfitz @rasky @randall77 @gopherbot