Skip to content

Commit ab2e3cc

Browse files
lizthegreyclaudeklauspost
authored
zstd: Add arm64 decoder asm (#1160)
* zstd: add arm64 asm implementation of fseDecoder.buildDtable arm64 previously fell through to the pure-Go fse_decoder_generic.go because the decoder asm is avo-generated and avo has no arm64 backend. This adds a hand-written scalar arm64 port of buildDtable, mirroring the avo-generated fse_decoder_amd64.s and the pure-Go reference. The kernel is serial bitfield work (no SIMD), so it needs no NEON/SVE and builds on the stock go1.x assembler. Correctness is covered by a differential test against an in-tree copy of the generic algorithm over the three RFC predefined distributions, plus the full existing decode suite (which runs on macos-latest/arm64 in CI). darwin/arm64 (M4, go1.26), asm vs -tags noasm; the bench's table reset is identical on both sides so the delta is the buildDtable work: litLength ~166 vs ~201 ns/op (+18%) offset ~118 vs ~141 ns/op (+16%) matchLength ~175 vs ~210 ns/op (+17%) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * zstd: add avo-generated arm64 asm sequence decoder Generates the full non-BMI2 zstd sequence decoder for arm64 by lowering the existing amd64 avo program (_generate/gen.go) through a new arm64 backend in avo (a post-pass.Compile lowering printer; see mmcloughlin/avo#486). No asm is hand-written: `go generate` now emits both seqdec_amd64.s and seqdec_arm64.s from the same IR, so they cannot drift, and CI's generate+git-diff job enforces it. - seqdec_arm64.s: generated (6 non-BMI2 functions: decode/_56, executeSimple/ _safe, decodeSync/_safe). - seqdec_arm64.go: shim mirroring seqdec_amd64.go (context structs, noescape decls, dispatch) without the BMI2 paths. - seqdec_generic.go: build tag now excludes arm64. Scalar only — no NEON/SVE (one VLD1/VST1 pair for a 16-byte MOVUPS copy); builds on go1.26. Full zstd decode suite passes on arm64. On Apple M4, BenchmarkDecoder_DecodeAll is ~1.5x faster than the pure-Go fallback on sequence-heavy input (kppkn: 469 vs 304 MB/s). Regeneration requires the avo arm64 backend (mmcloughlin/avo#486) via a local replace directive in _generate/go.mod (not committed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * zstd: generate arm64 fse decoder via avo + pin avo fork Switches fse_decoder_arm64.s from the hand-written bootstrap to the avo-generated lowering, so the entire arm64 zstd decoder (fse table build + sequence decoder) is produced from the existing, unmodified amd64 avo generators via the arm64 lowering printer (mmcloughlin/avo#486). Supersedes the hand-written fse PR #1159. Pins the printer through a replace to the honeycombio/avo fork (which keeps the github.com/mmcloughlin/avo module path, so the replace resolves) until the printer merges upstream and is released, at which point the replace is dropped for a normal require. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * zstd: asmfmt-style formatting for generated arm64 asm Regenerate the arm64 decoder with the updated avo arm64 printer, which now aligns operands to a common column and emits a blank line before labels — matching avo's amd64 (goasm) output. asmfmt reports zero non-comment differences on the arm64 files (the same standalone-comment indentation residual goasm produces on amd64). Repins the avo fork to the printer commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * zstd: address review — dedup shims, -arch generation, asm-vs-nonasm fuzz Responds to maintainer review on the arm64 decoder PR: - Deduplicate the asm Go wrappers: shared seqdec_asm.go holds the context structs, error codes and the decode/decodeSync/executeSimple bodies; each arch keeps only its //go:noescape decls and a small dispatch helper (amd64 selects the BMI2 / 56-bit / safe variant, arm64 the 56-bit / safe variant). The fse Go wrapper is identical across arches and collapses into one fse_decoder_asm.go (per-arch fse_decoder_{amd64,arm64}.go removed). - Generate both architectures via avo's new -arch flag: one `go run gen.go -out ../seqdec.s -arch amd64,arm64` writes seqdec_amd64.s and seqdec_arm64.s from a single compile, so neither file's header references the other architecture. - FuzzDecodeAll now cross-checks the assembly decoder against a non-assembly path (BMI2 disabled on amd64) for byte-identical output; run with -tags=noasm over the shared corpus to cover assembly-vs-generic on arm64. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: run build matrix and zstd fuzzers on linux arm64 Adds ubuntu-24.04-arm to the build matrix (vet/test/noasm/race across Go versions on Linux arm64) and gives the fuzz-zstd job an OS matrix so the zstd decode fuzzers also run on arm64 — exercising the new arm64 decoder assembly directly in CI, as requested in review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * zstd: defer BMI2 restore in FuzzDecodeAll asm/non-asm comparison Use the defer cpuinfo.DisableBMI2()() idiom so BMI2 is re-enabled on every exit path (panic/return), matching FuzzDecAllNoBMI2. Addresses review feedback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Klaus Post <klauspost@gmail.com>
1 parent bb2723d commit ab2e3cc

18 files changed

Lines changed: 3462 additions & 343 deletions

.github/workflows/go.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jobs:
1313
strategy:
1414
matrix:
1515
go-version: [1.24.x, 1.25.x, 1.26.x]
16-
os: [ubuntu-latest, macos-latest, windows-latest]
16+
os: [ubuntu-latest, ubuntu-24.04-arm, macos-latest, windows-latest]
1717
env:
1818
CGO_ENABLED: 0
1919
runs-on: ${{ matrix.os }}
@@ -146,9 +146,10 @@ jobs:
146146
fuzz-zstd:
147147
env:
148148
CGO_ENABLED: 0
149-
runs-on: ubuntu-latest
149+
runs-on: ${{ matrix.os }}
150150
strategy:
151151
matrix:
152+
os: [ ubuntu-latest, ubuntu-24.04-arm ]
152153
tags: [ 'nounsafe', '"noasm,nounsafe"' ]
153154
steps:
154155
- name: Set up Go

zstd/_generate/gen.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package main
22

3-
//go:generate go run gen.go -out ../seqdec_amd64.s -pkg=zstd
3+
//go:generate go run gen.go -out ../seqdec.s -arch amd64,arm64 -pkg=zstd
44

55
import (
66
"flag"

zstd/_generate/gen_fse.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package main
22

3-
//go:generate go run gen_fse.go -out ../fse_decoder_amd64.s -pkg=zstd
3+
//go:generate go run gen_fse.go -out ../fse_decoder.s -arch amd64,arm64 -pkg=zstd
44

55
import (
66
"flag"

zstd/_generate/go.mod

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ require (
1010
)
1111

1212
require (
13-
golang.org/x/mod v0.21.0 // indirect
14-
golang.org/x/sync v0.8.0 // indirect
15-
golang.org/x/tools v0.25.1 // indirect
13+
golang.org/x/mod v0.27.0 // indirect
14+
golang.org/x/sync v0.16.0 // indirect
15+
golang.org/x/tools v0.36.0 // indirect
1616
)
1717

1818
replace github.com/klauspost/compress => ../..
19+
20+
replace github.com/mmcloughlin/avo => github.com/honeycombio/avo v0.6.1-0.20260629192935-ff52dcb5f88d

zstd/_generate/go.sum

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
github.com/mmcloughlin/avo v0.6.0 h1:QH6FU8SKoTLaVs80GA8TJuLNkUYl4VokHKlPhVDg4YY=
2-
github.com/mmcloughlin/avo v0.6.0/go.mod h1:8CoAGaCSYXtCPR+8y18Y9aB/kxb8JSS6FRI7mSkvD+8=
3-
golang.org/x/mod v0.21.0 h1:vvrHzRwRfVKSiLrG+d4FMl/Qi4ukBCE6kZlTUkDYRT0=
4-
golang.org/x/mod v0.21.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
5-
golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
6-
golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
7-
golang.org/x/tools v0.25.1 h1:YeIyhd0M7gStYR9jb2IFXVVT+QJhgXu1ZECOuRwofh4=
8-
golang.org/x/tools v0.25.1/go.mod h1:/vtpO8WL1N9cQC3FN5zPqb//fRXskFHbLKk4OW1Q7rg=
1+
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
2+
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
3+
github.com/honeycombio/avo v0.6.1-0.20260629192935-ff52dcb5f88d h1:946aSI2nbyDYa8iTjFmwoM4S4TKmE8heVLGlchLj7vU=
4+
github.com/honeycombio/avo v0.6.1-0.20260629192935-ff52dcb5f88d/go.mod h1:+Pk+j3KceMwAIGb32HZSCvkMCzJvxy6xCRk+urz7iPw=
5+
golang.org/x/mod v0.27.0 h1:kb+q2PyFnEADO2IEF935ehFUXlWiNjJWtRNgBLSfbxQ=
6+
golang.org/x/mod v0.27.0/go.mod h1:rWI627Fq0DEoudcK+MBkNkCe0EetEaDSwJJkCcjpazc=
7+
golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw=
8+
golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
9+
golang.org/x/tools v0.36.0 h1:kWS0uv/zsvHEle1LbV5LE8QujrxB3wfQyxHfhOk0Qkg=
10+
golang.org/x/tools v0.36.0/go.mod h1:WBDiHKJK8YgLHlcQPYQzNCkUxUypCaa5ZegCVutKm+s=

zstd/fse_decoder_amd64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Code generated by command: go run gen_fse.go -out ../fse_decoder_amd64.s -pkg=zstd. DO NOT EDIT.
1+
// Code generated by command: go run gen_fse.go -out ../fse_decoder.s -arch amd64,arm64 -pkg=zstd. DO NOT EDIT.
22

33
//go:build !appengine && !noasm && gc && !noasm
44

zstd/fse_decoder_arm64.s

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
// Code generated by command: go run gen_fse.go -out ../fse_decoder.s -arch amd64,arm64 -pkg=zstd. DO NOT EDIT.
2+
// EXPERIMENTAL arm64 output lowered from an amd64 avo program.
3+
4+
//go:build arm64 && !appengine && !noasm && gc && !noasm
5+
6+
// func buildDtable_asm(s *fseDecoder, ctx *buildDtableAsmContext) int
7+
TEXT ·buildDtable_asm(SB), $0-24
8+
MOVD ctx+8(FP), R1
9+
MOVD s+0(FP), R6
10+
11+
// Load values
12+
MOVBU 4098(R6), R2
13+
MOVD $0, R0
14+
MOVD $1, R16
15+
LSL R2, R16, R16
16+
ORR R16, R0, R0
17+
MOVD (R1), R3
18+
MOVD 16(R1), R5
19+
SUB $1, R0, R7
20+
MOVD 8(R1), R1
21+
MOVHU 4096(R6), R6
22+
23+
// End load values
24+
// Init, lay down lowprob symbols
25+
MOVD $0, R8
26+
JMP init_main_loop_condition
27+
28+
init_main_loop:
29+
ADD R8<<1, R1, R15
30+
MOVH (R15), R9
31+
CMN $1, R9
32+
BNE do_not_update_high_threshold
33+
ADD R7<<3, R5, R15
34+
MOVB R8, 1(R15)
35+
SUB $1, R7, R7
36+
MOVD $0x0000000000000001, R9
37+
38+
do_not_update_high_threshold:
39+
ADD R8<<1, R3, R15
40+
MOVH R9, (R15)
41+
ADD $1, R8, R8
42+
43+
init_main_loop_condition:
44+
CMP R6, R8
45+
BLT init_main_loop
46+
47+
// Spread symbols
48+
// Calculate table step
49+
MOVD R0, R8
50+
LSR $0x01, R8, R8
51+
MOVD R0, R9
52+
LSR $0x03, R9, R9
53+
ADD R9, R8, R8
54+
ADD $3, R8, R8
55+
56+
// Fill add bits values
57+
SUB $1, R0, R9
58+
MOVD $0, R10
59+
MOVD $0, R11
60+
JMP spread_main_loop_condition
61+
62+
spread_main_loop:
63+
MOVD $0, R12
64+
ADD R11<<1, R1, R15
65+
MOVH (R15), R13
66+
JMP spread_inner_loop_condition
67+
68+
spread_inner_loop:
69+
ADD R10<<3, R5, R15
70+
MOVB R11, 1(R15)
71+
72+
adjust_position:
73+
ADD R8, R10, R10
74+
AND R9, R10, R10
75+
CMP R7, R10
76+
BGT adjust_position
77+
ADD $1, R12, R12
78+
79+
spread_inner_loop_condition:
80+
CMP R13, R12
81+
BLT spread_inner_loop
82+
ADD $1, R11, R11
83+
84+
spread_main_loop_condition:
85+
CMP R6, R11
86+
BLT spread_main_loop
87+
TST R10, R10
88+
BEQ spread_check_ok
89+
MOVD ctx+8(FP), R0
90+
MOVD R10, 24(R0)
91+
MOVD $+1, R16
92+
MOVD R16, ret+16(FP)
93+
RET
94+
95+
spread_check_ok:
96+
// Build Decoding table
97+
MOVD $0, R6
98+
99+
build_table_main_table:
100+
ADD R6<<3, R5, R15
101+
MOVBU 1(R15), R1
102+
ADD R1<<1, R3, R15
103+
MOVHU (R15), R7
104+
ADD $1, R7, R8
105+
ADD R1<<1, R3, R15
106+
MOVH R8, (R15)
107+
MOVD R7, R8
108+
CLZ R8, R16
109+
MOVD $63, R8
110+
SUB R16, R8, R8
111+
MOVD R2, R1
112+
SUB R8, R1, R1
113+
LSL R1, R7, R7
114+
SUB R0, R7, R7
115+
ADD R6<<3, R5, R15
116+
MOVB R1, (R15)
117+
ADD R6<<3, R5, R15
118+
MOVH R7, 2(R15)
119+
CMP R0, R7
120+
BLE build_table_check1_ok
121+
MOVD ctx+8(FP), R1
122+
MOVD R7, 24(R1)
123+
MOVD R0, 32(R1)
124+
MOVD $+2, R16
125+
MOVD R16, ret+16(FP)
126+
RET
127+
128+
build_table_check1_ok:
129+
TST R1, R1
130+
BNE build_table_check2_ok
131+
CMP R6, R7
132+
BNE build_table_check2_ok
133+
MOVD ctx+8(FP), R0
134+
MOVD R7, 24(R0)
135+
MOVD R6, 32(R0)
136+
MOVD $+3, R16
137+
MOVD R16, ret+16(FP)
138+
RET
139+
140+
build_table_check2_ok:
141+
ADD $1, R6, R6
142+
CMP R0, R6
143+
BLT build_table_main_table
144+
MOVD $+0, R16
145+
MOVD R16, ret+16(FP)
146+
RET

zstd/fse_decoder_arm64_test.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
//go:build arm64 && !appengine && !noasm && gc
2+
3+
package zstd
4+
5+
import (
6+
"errors"
7+
"fmt"
8+
"reflect"
9+
"testing"
10+
)
11+
12+
// buildDtableRef is a copy of the pure-Go reference algorithm
13+
// (fse_decoder_generic.go), kept here so the arm64 asm implementation can be
14+
// differentially tested against it even though the generic file is not
15+
// compiled on arm64.
16+
func buildDtableRef(s *fseDecoder) error {
17+
tableSize := uint32(1 << s.actualTableLog)
18+
highThreshold := tableSize - 1
19+
symbolNext := s.stateTable[:256]
20+
21+
{
22+
for i, v := range s.norm[:s.symbolLen] {
23+
if v == -1 {
24+
s.dt[highThreshold].setAddBits(uint8(i))
25+
highThreshold--
26+
v = 1
27+
}
28+
symbolNext[i] = uint16(v)
29+
}
30+
}
31+
32+
{
33+
tableMask := tableSize - 1
34+
step := tableStep(tableSize)
35+
position := uint32(0)
36+
for ss, v := range s.norm[:s.symbolLen] {
37+
for i := 0; i < int(v); i++ {
38+
s.dt[position].setAddBits(uint8(ss))
39+
for {
40+
position = (position + step) & tableMask
41+
if position <= highThreshold {
42+
break
43+
}
44+
}
45+
}
46+
}
47+
if position != 0 {
48+
return errors.New("corrupted input (position != 0)")
49+
}
50+
}
51+
52+
{
53+
tableSize := uint16(1 << s.actualTableLog)
54+
for u, v := range s.dt[:tableSize] {
55+
symbol := v.addBits()
56+
nextState := symbolNext[symbol]
57+
symbolNext[symbol] = nextState + 1
58+
nBits := s.actualTableLog - byte(highBits(uint32(nextState)))
59+
s.dt[u&maxTableMask].setNBits(nBits)
60+
newState := (nextState << nBits) - tableSize
61+
if newState > tableSize {
62+
return fmt.Errorf("newState (%d) outside table size (%d)", newState, tableSize)
63+
}
64+
if newState == uint16(u) && nBits == 0 {
65+
return fmt.Errorf("newState (%d) == oldState (%d) and no bits", newState, u)
66+
}
67+
s.dt[u&maxTableMask].setNewState(newState)
68+
}
69+
}
70+
return nil
71+
}
72+
73+
func TestBuildDtableARM64MatchesReference(t *testing.T) {
74+
for name, mk := range predefinedFSEInputs() {
75+
t.Run(name, func(t *testing.T) {
76+
got := mk()
77+
if err := got.buildDtable(); err != nil {
78+
t.Fatalf("asm buildDtable: %v", err)
79+
}
80+
want := mk()
81+
if err := buildDtableRef(want); err != nil {
82+
t.Fatalf("reference buildDtable: %v", err)
83+
}
84+
tableSize := 1 << got.actualTableLog
85+
if !reflect.DeepEqual(got.dt[:tableSize], want.dt[:tableSize]) {
86+
for i := range tableSize {
87+
if got.dt[i] != want.dt[i] {
88+
t.Errorf("dt[%d]: asm %#016x, ref %#016x", i, uint64(got.dt[i]), uint64(want.dt[i]))
89+
}
90+
}
91+
t.FailNow()
92+
}
93+
if got.stateTable != want.stateTable {
94+
t.Errorf("stateTable mismatch:\nasm %v\nref %v", got.stateTable, want.stateTable)
95+
}
96+
})
97+
}
98+
}
Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
1-
//go:build amd64 && !appengine && !noasm && gc
1+
//go:build (amd64 || arm64) && !appengine && !noasm && gc
22

33
package zstd
44

55
import (
66
"fmt"
77
)
88

9+
// buildDtable_asm is generated by _generate/gen_fse.go and lowered to each
10+
// architecture (amd64 by goasm, arm64 by the avo arm64 lowering printer). The
11+
// Go side is identical across architectures, so it lives here.
12+
913
type buildDtableAsmContext struct {
1014
// inputs
1115
stateTable *uint16
@@ -18,7 +22,7 @@ type buildDtableAsmContext struct {
1822
errParam2 uint64
1923
}
2024

21-
// buildDtable_asm is an x86 assembly implementation of fseDecoder.buildDtable.
25+
// buildDtable_asm is an assembly implementation of fseDecoder.buildDtable.
2226
// Function returns non-zero exit code on error.
2327
//
2428
//go:noescape

0 commit comments

Comments
 (0)