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/compile: missed peephole optimizations #10637

Closed
brtzsnr opened this issue Apr 30, 2015 · 4 comments
Closed

cmd/compile: missed peephole optimizations #10637

brtzsnr opened this issue Apr 30, 2015 · 4 comments
Labels
FrozenDueToAge

Comments

@brtzsnr
Copy link
Contributor

brtzsnr commented Apr 30, 2015 

func S16(a uint64) uint64 {
        return a << 8 << 8 << 0
}

The generated assembly is:

"".S16 t=1 size=32 value=0 args=0x10 locals=0x0
        0x0000 00000 (source/test/a.go:3)  TEXT    "".S16(SB), $0-16
        0x0000 00000 (source/test/a.go:3)  NOP
        0x0000 00000 (source/test/a.go:3)  NOP
        0x0000 00000 (source/test/a.go:3)  FUNCDATA        $0, gclocals·2fccd208efe70893f9ac8d682812ae72(SB)
        0x0000 00000 (source/test/a.go:3)  FUNCDATA        $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
        0x0000 00000 (source/test/a.go:4)  MOVQ    "".a+8(FP), BX
        0x0005 00005 (source/test/a.go:4)  SHRQ    $8, BX
        0x0009 00009 (source/test/a.go:4)  SHRQ    $8, BX
        0x000d 00013 (source/test/a.go:4)  SHRQ    $0, BX
        0x0011 00017 (source/test/a.go:4)  MOVQ    BX, "".~r1+16(FP)
        0x0016 00022 (source/test/a.go:4)  RET

S16 should generate a single right shift (SHRQ), not three. Moreover, SHRQ $0, BX is a nop and shouldn't be generated at all. Same goes for SHLQ, ADDQ, SUBQ.

For multiplication is a bit better, but deadstores are not detected.

func M1(a uint64) uint64 {
        return a * 8 * 8 * 1
}
func M2(a uint64) uint64 {
        return a * 8 * 8 * 0
}

Generates the following assembly:

"".M1 t=1 size=32 value=0 args=0x10 locals=0x0
        0x0000 00000 (source/test/a.go:7)  TEXT    "".M1(SB), $0-16
        0x0000 00000 (source/test/a.go:7)  NOP
        0x0000 00000 (source/test/a.go:7)  NOP
        0x0000 00000 (source/test/a.go:7)  FUNCDATA        $0, gclocals·2fccd208efe70893f9ac8d682812ae72(SB)
        0x0000 00000 (source/test/a.go:7)  FUNCDATA        $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
        0x0000 00000 (source/test/a.go:8)  MOVQ    "".a+8(FP), BX
        0x0005 00005 (source/test/a.go:8)  SHLQ    $3, BX
        0x0009 00009 (source/test/a.go:8)  SHLQ    $3, BX
        0x000d 00013 (source/test/a.go:8)  MOVQ    BX, "".~r1+16(FP)
        0x0012 00018 (source/test/a.go:8)  RET

"".M2 t=1 size=32 value=0 args=0x10 locals=0x0
        0x0000 00000 (source/test/a.go:10) TEXT    "".M2(SB), $0-16
        0x0000 00000 (source/test/a.go:10) NOP
        0x0000 00000 (source/test/a.go:10) NOP
        0x0000 00000 (source/test/a.go:10) FUNCDATA        $0, gclocals·2fccd208efe70893f9ac8d682812ae72(SB)
        0x0000 00000 (source/test/a.go:10) FUNCDATA        $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
        0x0000 00000 (source/test/a.go:11) MOVQ    "".a+8(FP), BX
        0x0005 00005 (source/test/a.go:11) SHLQ    $3, BX
        0x0009 00009 (source/test/a.go:11) SHLQ    $3, BX
        0x000d 00013 (source/test/a.go:11) MOVQ    $0, "".~r1+16(FP)
        0x0016 00022 (source/test/a.go:11) RET

Multiplication with 0 is eliminated, but the dead stores are not.
Multiplication with 1 is eliminated, but the two multiplications by 8 are not coalesced.

Everything was build with the latest version.

% go version
go version devel +cfb8b18 Thu Apr 30 08:33:29 2015 +0000 linux/amd64
@brtzsnr brtzsnr changed the title Missed peephole optimization for shifting Missed peephole optimizations Apr 30, 2015
@josharian
Copy link
Contributor

/cc @randall77 for SSA

@josharian josharian changed the title Missed peephole optimizations cmd/internal/gc: missed peephole optimizations Apr 30, 2015
@randall77 randall77 self-assigned this Apr 30, 2015
@randall77
Copy link
Contributor

Yes, this will be easy in SSA. Planned for 1.6.

@randall77 randall77 added this to the Go1.6 milestone May 20, 2015
@brtzsnr brtzsnr mentioned this issue Aug 5, 2015
Closed
@ianlancetaylor ianlancetaylor changed the title cmd/internal/gc: missed peephole optimizations cmd/compile: missed peephole optimizations Sep 3, 2015
@rsc rsc modified the milestones: dev.ssa, Go1.6 Nov 4, 2015
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Feb 4, 2016
@tzneal
[dev.ssa] cmd/compile: rewrites for constant shifts
93a0b0f 
Add rewrite rules to optimize constant shifts.

Fixes #10637

Change-Id: I74b724d3e81aeb7098c696d02c050f7fdfd5b523
Reviewed-on: https://go-review.googlesource.com/19106
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@randall77
Copy link
Contributor

This is fixed on tip SSA.

@golang golang locked and limited conversation to collaborators Feb 28, 2017
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
5 participants
@brtzsnr @josharian @rsc @randall77 @gopherbot