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

cmd/compile: unnecessary zeroing of AX after MOVBLZX #25378

Closed
davecheney opened this issue May 14, 2018 · 3 comments
Closed

cmd/compile: unnecessary zeroing of AX after MOVBLZX #25378

davecheney opened this issue May 14, 2018 · 3 comments

Comments

@davecheney
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

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

% go version
go version devel +b7f3c178a3 Mon May 14 04:42:45 2018 +0000 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

darwin

What did you do?

This function

var wsp = [256]bool{
        ' ':  true,
        '\t': true,
        '\n': true,
        '\r': true,
}

func isWhitespace(ch byte) bool { return wsp[ch] }

What did you expect to see?

Something like MOVBLZX into AL

"".isWhitespace STEXT nosplit size=24 args=0x10 locals=0x0
        0x0000 00000 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       TEXT    "".isWhitespace(SB), NOSPLIT, $0-16
        0x0000 00000 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       FUNCDATA        $0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
        0x0000 00000 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       FUNCDATA        $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
        0x0000 00000 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       MOVBLZX "".ch+8(SP), AL
         // elided 0x0005 00005 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       MOVBLZX AL, AX
        0x0008 00008 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       LEAQ    "".wsp(SB), CX
        0x000f 00015 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       MOVBLZX (CX)(AX*1), AX
        0x0013 00019 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       MOVB    AL, "".~r1+16(SP)
        0x0017 00023 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       RET
        0x0000 0f b6 44 24 08 0f b6 c0 48 8d 0d 00 00 00 00 0f  ..D$....H.......
        0x0010 b6 04 01 88 44 24 10 c3                          ....D$..
        rel 11+4 t=15 "".wsp+0

What did you see instead?

Instead we get

"".isWhitespace STEXT nosplit size=24 args=0x10 locals=0x0
        0x0000 00000 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       TEXT    "".isWhitespace(SB), NOSPLIT, $0-16
        0x0000 00000 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       FUNCDATA        $0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
        0x0000 00000 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       FUNCDATA        $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
        0x0000 00000 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       MOVBLZX "".ch+8(SP), AX
        0x0005 00005 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       MOVBLZX AL, AX
        0x0008 00008 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       LEAQ    "".wsp(SB), CX
        0x000f 00015 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       MOVBLZX (CX)(AX*1), AX
        0x0013 00019 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       MOVB    AL, "".~r1+16(SP)
        0x0017 00023 (/Users/dfc/src/github.com/davecheney/whitesp/whitesp.go:14)       RET
        0x0000 0f b6 44 24 08 0f b6 c0 48 8d 0d 00 00 00 00 0f  ..D$....H.......
        0x0010 b6 04 01 88 44 24 10 c3                          ....D$..
        rel 11+4 t=15 "".wsp+0
@martisch
Copy link
Contributor

martisch commented May 14, 2018

dup or variant of #15300?

I dont think a mov zero extend of 8bit into an 8bit register (AL) exist on amd64. But combining the two moves into just one MOVBLZX "".ch+8(SP), AX would as you note suffice here.

@davecheney
Copy link
Contributor Author

davecheney commented May 14, 2018 via email

@josharian josharian added this to the Unplanned milestone May 16, 2018
@gopherbot
Copy link

Change https://golang.org/cl/115617 mentions this issue: cmd/compile/internal/ssa: remove useless zero extension

@golang golang locked and limited conversation to collaborators Aug 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants