golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.85k stars 17.65k forks source link

cmd/compile: Unnecessary register high-bytes rezeroing after unsigned right shift #19712

Open navytux opened 7 years ago

navytux commented 7 years ago

Please answer these questions before submitting your issue. Thanks!

What did you do?

Hello up there. Please consider the following function:

package xxx

const hextable = "0123456789abcdef"

func HexHi(v byte) byte {
        return hextable[v>>4]
}

(https://play.golang.org/p/6t7Bq7lw2k)

it compiles to the following assembly:

#include "funcdata.h"

TEXT ·HexHi(SB), $0-16 // bbb.go:5
        NO_LOCAL_POINTERS
        // FUNCDATA $0, gclocals·f207267fbf96a0178e8758c6e3e0ce28(SB) (args)
        // FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB) (no locals)
        MOVBLZX    v+0(FP), AX  // bbb.go:6
        SHRB       $4, AL
        MOVBLZX    AL, AX                                 <-- NOTE
        LEAQ       go.string."0123456789abcdef"(SB), CX
        MOVBLZX    (CX)(AX*1), AX
        MOVB       AL, _r1+8(FP)
        RET

where:

  1. v was byte-zero-extend loaded from stack argument MOVBLZX v+0(FP), AX
  2. v was shifted right by 4 SHRB $4, AL

and then

  1. compiler makes sure that higher bytes of AX are zero MOVBLZX AL, AX

Step 3 is not neccessary since original MOVBLZX v+0(FP), AX is zero-extend move and after it AX[31:8] is zero and logical right shifts preserve that property (even more: for unsigned v, i if we know at some time that v is <= A, for v>>i we can know that it is <= A/2^i).

What did you expect to see?

No high-bytes rezeroing after right shift on zero-extended byte-loaded register.

What did you see instead?

Unneccessary high-bytes rezeroing instruction emitted.

Does this issue reproduce with the latest release (go1.8)?

Yes.

Possibly related issues

18575

System details

go version devel +ecc6a81617 Sat Mar 25 00:35:35 2017 +0000 linux/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/kirr/go"
GORACE=""
GOROOT="/home/kirr/src/tools/go/go"
GOTOOLDIR="/home/kirr/src/tools/go/go/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build279220346=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOROOT/bin/go version: go version devel +ecc6a81617 Sat Mar 25 00:35:35 2017 +0000 linux/amd64
GOROOT/bin/go tool compile -V: compile version devel +ecc6a81617 Sat Mar 25 00:35:35 2017 +0000 X:framepointer
uname -sr: Linux 4.9.0-2-amd64
Distributor ID: Debian
Description:    Debian GNU/Linux 9.0 (stretch)
Release:    9.0
Codename:   stretch
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Debian GLIBC 2.24-9) stable release version 2.24, by Roland McGrath et al.
gdb --version: GNU gdb (Debian 7.12-6) 7.12.0.20161007-git

Thanks beforehand, Kirill

/cc @randall77

randall77 commented 7 years ago

Probably a dup of #15300