search

golang / snappy

The Snappy compression format in the Go programming language.
BSD 3-Clause "New" or "Revised" License
1.52k stars 163 forks source link

Fix wrong arm64 scaled register format #61

Closed cuonglm closed 3 years ago

cuonglm commented 3 years ago

Arm64 does not have scaled register format, casue snappy test failed for current go tip:

    $ go version
    go version devel go1.17-24875e3880 Tue Apr 20 15:14:05 2021 +0000 darwin/arm64
    $ go test
    # github.com/golang/snappy
    ./encode_arm64.s:385: arm64 doesn't support scaled register format
    ./encode_arm64.s:675: arm64 doesn't support scaled register format
    asm: assembly of ./encode_arm64.s failed
    FAIL    github.com/golang/snappy [build failed]

See https://go-review.googlesource.com/c/go/+/289589

cuonglm commented 3 years ago

Kindly ping @nigeltao

nigeltao commented 3 years ago

@AWSjswinney can you have a look?

AWSjswinney commented 3 years ago

As expected, the assembler in the 1.17 candidate emits the exact same instruction as previous versions (same for both changed lines):

b8af68c4        ldrsw   x4, [x6, x15]

Looks good to me.

AWSjswinney commented 3 years ago

@nigeltao You might want to tag a release after this merge because this issue is going to become mainstream as soon as projects start building with Go 1.17, due in about a month.

cuonglm commented 3 years ago

@AWSjswinney @nigeltao it's not necessary anymore, since when Cherry decide to restore supporting for *1 syntax, see https://go-review.googlesource.com/c/go/+/328229

AWSjswinney commented 3 years ago

Great! Thanks for pointing that out.

nigeltao commented 3 years ago

It might not be necessary, but it was easy to tag v0.0.4