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

Comments

Projects
None yet
4 participants
@davecheney
Copy link
Contributor

commented May 14, 2018

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2018

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

@gopherbot

This comment has been minimized.

Copy link

commented May 31, 2018

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

@gopherbot gopherbot closed this in a359368 Aug 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.