This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-go.git
The following commit(s) were added to refs/heads/main by this push:
new fa5d0bc0 fix(arrow/compute): correct amd64 kernel assembly ABI (#1432)
fa5d0bc0 is described below
commit fa5d0bc036508a4298e84c0eefaa8bd511ac207d
Author: Matt Topol <[email protected]>
AuthorDate: Thu Oct 29 16:09:59 2026 +0000
fix(arrow/compute): correct amd64 kernel assembly ABI (#1432)
### Rationale for this change
`go vet ./arrow/compute/internal/kernels` reports argument-size
mismatches and clobbered frame pointers in the generated amd64
arithmetic and numeric-cast kernels.
The arithmetic entry points declare a signed `int8` operation but
c2goasm loads a full word. The same generated functions and the
numeric-cast entry points also use BP for their constant tables without
allocating a Go stack frame to preserve the caller's BP.
### What changes are included in this PR?
- Load arithmetic operation arguments with `MOVBQSX`, matching the Go
`int8` and C++ `int8_t` declarations without changing their ABI or call
sites.
- Allocate a small nonzero Go frame for the 12 affected arithmetic/cast
entry points so the Go assembler saves and restores BP.
- Apply both corrections in the existing Makefile generation recipes,
with comments explaining why they are needed. The assembly bodies and
numeric algorithms are unchanged.
### Are these changes tested?
On linux/amd64 with Go 1.27.3:
- `go vet ./arrow/compute/internal/kernels` — passes with no output.
- `go test ./arrow/compute/... -count=1` — passes for all four packages.
- `go test ./arrow/compute -run
'^(TestBinaryArithmetic|TestUnaryArithmetic|TestCasts)$' -count=1` —
passes using the default AVX2 dispatch.
- `GODEBUG=cpu.avx2=off go test ./arrow/compute -run
'^(TestBinaryArithmetic|TestUnaryArithmetic|TestCasts)$' -count=1` —
passes using SSE4 dispatch.
- Dry-ran the four Makefile targets, then applied their expanded
post-processing recipes to the upstream assembly in memory: each result
exactly matches the checked-in changes, and applying the fixups again is
idempotent. Full C++/c2goasm regeneration was not run because those
tools are not installed.
The initial full-repository `go vet ./...` also found existing issues
outside this package: BP clobbers in `internal/utils` and
`parquet/internal/bmi`, unsafe-pointer diagnostics in `arrow/cdata` and
`arrow/memory/mallocator`, lock-copy diagnostics in `internal/types` and
the flight integration code, and an unkeyed benchmark literal in
`parquet/internal/utils`. Those are intentionally outside this PR's
scope. CI currently runs `go test` and staticcheck but has no explicit
full `go vet` step or enabled golangci-lint `govet` analyzer.
### Are there any user-facing changes?
No public API or arithmetic/cast result changes. The amd64 assembly now
obeys the declared argument widths and preserves the caller's frame
pointer.
---
arrow/compute/internal/kernels/Makefile | 11 +++++++++++
.../internal/kernels/base_arithmetic_avx2_amd64.s | 20 ++++++++++----------
.../internal/kernels/base_arithmetic_sse4_amd64.s | 20 ++++++++++----------
.../internal/kernels/cast_numeric_avx2_amd64.s | 2 +-
.../internal/kernels/cast_numeric_sse4_amd64.s | 2 +-
5 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/arrow/compute/internal/kernels/Makefile
b/arrow/compute/internal/kernels/Makefile
index c8c5bb66..c0dd49ce 100644
--- a/arrow/compute/internal/kernels/Makefile
+++ b/arrow/compute/internal/kernels/Makefile
@@ -16,6 +16,11 @@
# this converts rotate instructions from "ro[lr] <reg>" -> "ro[lr] <reg>, 1"
for yasm compatibility
PERL_FIXUP_ROTATE=perl -i -pe 's/(ro[rl]\s+\w{2,3})$$/\1, 1/'
+# c2goasm loads whole words, but the arithmetic operation is a signed int8.
+PERL_FIXUP_ARITHMETIC_OP=perl -i -pe 's/MOVQ (op\+(?:8|16)\(FP\))/MOVBQSX \1/'
+# A nonzero Go frame makes the assembler save and restore BP, which c2goasm
+# uses to address constants in otherwise frameless arithmetic and cast kernels.
+PERL_FIXUP_FRAME_POINTER=perl -i -pe 's/^(TEXT .*, \$$)0-/$${1}8-/'
C2GOASM=c2goasm
CC=clang-11
@@ -106,9 +111,11 @@ _lib/constant_factor_neon.s: _lib/constant_factor_neon.c
cast_numeric_avx2_amd64.s: _lib/cast_numeric_avx2_amd64.s
$(C2GOASM) -a -f $^ $@
+ $(PERL_FIXUP_FRAME_POINTER) $@
cast_numeric_sse4_amd64.s: _lib/cast_numeric_sse4_amd64.s
$(C2GOASM) -a -f $^ $@
+ $(PERL_FIXUP_FRAME_POINTER) $@
constant_factor_avx2_amd64.s: _lib/constant_factor_avx2_amd64.s
$(C2GOASM) -a -f $^ $@
@@ -118,9 +125,13 @@ constant_factor_sse4_amd64.s:
_lib/constant_factor_sse4_amd64.s
base_arithmetic_avx2_amd64.s: _lib/base_arithmetic_avx2_amd64.s
$(C2GOASM) -a -f $^ $@
+ $(PERL_FIXUP_ARITHMETIC_OP) $@
+ $(PERL_FIXUP_FRAME_POINTER) $@
base_arithmetic_sse4_amd64.s: _lib/base_arithmetic_sse4_amd64.s
$(C2GOASM) -a -f $^ $@
+ $(PERL_FIXUP_ARITHMETIC_OP) $@
+ $(PERL_FIXUP_FRAME_POINTER) $@
scalar_comparison_avx2_amd64.s: _lib/scalar_comparison_avx2_amd64.s
$(C2GOASM) -a -f $^ $@
diff --git a/arrow/compute/internal/kernels/base_arithmetic_avx2_amd64.s
b/arrow/compute/internal/kernels/base_arithmetic_avx2_amd64.s
index 420b64d5..86590e11 100644
--- a/arrow/compute/internal/kernels/base_arithmetic_avx2_amd64.s
+++ b/arrow/compute/internal/kernels/base_arithmetic_avx2_amd64.s
@@ -7,10 +7,10 @@ DATA LCDATA1<>+0x010(SB)/8, $0x00ff00ff00ff00ff
DATA LCDATA1<>+0x018(SB)/8, $0x00ff00ff00ff00ff
GLOBL LCDATA1<>(SB), 8, $32
-TEXT ·_arithmetic_binary_avx2(SB), $0-48
+TEXT ·_arithmetic_binary_avx2(SB), $8-48
MOVQ typ+0(FP), DI
- MOVQ op+8(FP), SI
+ MOVBQSX op+8(FP), SI
MOVQ inLeft+16(FP), DX
MOVQ inRight+24(FP), CX
MOVQ out+32(FP), R8
@@ -5847,10 +5847,10 @@ DATA LCDATA2<>+0x010(SB)/8, $0x00ff00ff00ff00ff
DATA LCDATA2<>+0x018(SB)/8, $0x00ff00ff00ff00ff
GLOBL LCDATA2<>(SB), 8, $32
-TEXT ·_arithmetic_arr_scalar_avx2(SB), $0-48
+TEXT ·_arithmetic_arr_scalar_avx2(SB), $8-48
MOVQ typ+0(FP), DI
- MOVQ op+8(FP), SI
+ MOVBQSX op+8(FP), SI
MOVQ inLeft+16(FP), DX
MOVQ inRight+24(FP), CX
MOVQ out+32(FP), R8
@@ -13311,10 +13311,10 @@ DATA LCDATA3<>+0x010(SB)/8, $0x00ff00ff00ff00ff
DATA LCDATA3<>+0x018(SB)/8, $0x00ff00ff00ff00ff
GLOBL LCDATA3<>(SB), 8, $32
-TEXT ·_arithmetic_scalar_arr_avx2(SB), $0-48
+TEXT ·_arithmetic_scalar_arr_avx2(SB), $8-48
MOVQ typ+0(FP), DI
- MOVQ op+8(FP), SI
+ MOVBQSX op+8(FP), SI
MOVQ inLeft+16(FP), DX
MOVQ inRight+24(FP), CX
MOVQ out+32(FP), R8
@@ -20559,10 +20559,10 @@ DATA LCDATA4<>+0x0b0(SB)/8, $0x1d1c191815141110
DATA LCDATA4<>+0x0b8(SB)/8, $0x1f1e1d1c1d1c1918
GLOBL LCDATA4<>(SB), 8, $192
-TEXT ·_arithmetic_unary_same_types_avx2(SB), $0-40
+TEXT ·_arithmetic_unary_same_types_avx2(SB), $8-40
MOVQ typ+0(FP), DI
- MOVQ op+8(FP), SI
+ MOVBQSX op+8(FP), SI
MOVQ input+16(FP), DX
MOVQ output+24(FP), CX
MOVQ len+32(FP), R8
@@ -25876,11 +25876,11 @@ DATA LCDATA5<>+0x0f0(SB)/8, $0x0101010101010101
DATA LCDATA5<>+0x0f8(SB)/8, $0x0101010101010101
GLOBL LCDATA5<>(SB), 8, $256
-TEXT ·_arithmetic_unary_diff_type_avx2(SB), $0-48
+TEXT ·_arithmetic_unary_diff_type_avx2(SB), $8-48
MOVQ itype+0(FP), DI
MOVQ otype+8(FP), SI
- MOVQ op+16(FP), DX
+ MOVBQSX op+16(FP), DX
MOVQ input+24(FP), CX
MOVQ output+32(FP), R8
MOVQ len+40(FP), R9
diff --git a/arrow/compute/internal/kernels/base_arithmetic_sse4_amd64.s
b/arrow/compute/internal/kernels/base_arithmetic_sse4_amd64.s
index 82c279eb..01211679 100644
--- a/arrow/compute/internal/kernels/base_arithmetic_sse4_amd64.s
+++ b/arrow/compute/internal/kernels/base_arithmetic_sse4_amd64.s
@@ -5,10 +5,10 @@ DATA LCDATA1<>+0x000(SB)/8, $0x00ff00ff00ff00ff
DATA LCDATA1<>+0x008(SB)/8, $0x00ff00ff00ff00ff
GLOBL LCDATA1<>(SB), 8, $16
-TEXT ·_arithmetic_binary_sse4(SB), $0-48
+TEXT ·_arithmetic_binary_sse4(SB), $8-48
MOVQ typ+0(FP), DI
- MOVQ op+8(FP), SI
+ MOVBQSX op+8(FP), SI
MOVQ inLeft+16(FP), DX
MOVQ inRight+24(FP), CX
MOVQ out+32(FP), R8
@@ -7198,10 +7198,10 @@ DATA LCDATA2<>+0x000(SB)/8, $0x00ff00ff00ff00ff
DATA LCDATA2<>+0x008(SB)/8, $0x00ff00ff00ff00ff
GLOBL LCDATA2<>(SB), 8, $16
-TEXT ·_arithmetic_arr_scalar_sse4(SB), $0-48
+TEXT ·_arithmetic_arr_scalar_sse4(SB), $8-48
MOVQ typ+0(FP), DI
- MOVQ op+8(FP), SI
+ MOVBQSX op+8(FP), SI
MOVQ inLeft+16(FP), DX
MOVQ inRight+24(FP), CX
MOVQ out+32(FP), R8
@@ -13789,10 +13789,10 @@ DATA LCDATA3<>+0x000(SB)/8, $0x00ff00ff00ff00ff
DATA LCDATA3<>+0x008(SB)/8, $0x00ff00ff00ff00ff
GLOBL LCDATA3<>(SB), 8, $16
-TEXT ·_arithmetic_scalar_arr_sse4(SB), $0-48
+TEXT ·_arithmetic_scalar_arr_sse4(SB), $8-48
MOVQ typ+0(FP), DI
- MOVQ op+8(FP), SI
+ MOVBQSX op+8(FP), SI
MOVQ inLeft+16(FP), DX
MOVQ inRight+24(FP), CX
MOVQ out+32(FP), R8
@@ -20499,10 +20499,10 @@ DATA LCDATA4<>+0x098(SB)/8, $0x000000ff000000ff
DATA LCDATA4<>+0x0a0(SB)/8, $0x3ff0000000000000
GLOBL LCDATA4<>(SB), 8, $168
-TEXT ·_arithmetic_unary_same_types_sse4(SB), $0-40
+TEXT ·_arithmetic_unary_same_types_sse4(SB), $8-40
MOVQ typ+0(FP), DI
- MOVQ op+8(FP), SI
+ MOVBQSX op+8(FP), SI
MOVQ input+16(FP), DX
MOVQ output+24(FP), CX
MOVQ len+32(FP), R8
@@ -26370,11 +26370,11 @@ DATA LCDATA5<>+0x128(SB)/8, $0x5f0000003f800000
DATA LCDATA5<>+0x130(SB)/8, $0x00000000bf800000
GLOBL LCDATA5<>(SB), 8, $312
-TEXT ·_arithmetic_unary_diff_type_sse4(SB), $0-48
+TEXT ·_arithmetic_unary_diff_type_sse4(SB), $8-48
MOVQ itype+0(FP), DI
MOVQ otype+8(FP), SI
- MOVQ op+16(FP), DX
+ MOVBQSX op+16(FP), DX
MOVQ input+24(FP), CX
MOVQ output+32(FP), R8
MOVQ len+40(FP), R9
diff --git a/arrow/compute/internal/kernels/cast_numeric_avx2_amd64.s
b/arrow/compute/internal/kernels/cast_numeric_avx2_amd64.s
index ffedf7a4..4d522dd9 100644
--- a/arrow/compute/internal/kernels/cast_numeric_avx2_amd64.s
+++ b/arrow/compute/internal/kernels/cast_numeric_avx2_amd64.s
@@ -27,7 +27,7 @@ DATA LCDATA1<>+0x0b0(SB)/8, $0x00ff00ff00ff00ff
DATA LCDATA1<>+0x0b8(SB)/8, $0x00ff00ff00ff00ff
GLOBL LCDATA1<>(SB), 8, $192
-TEXT ·_cast_type_numeric_avx2(SB), $0-40
+TEXT ·_cast_type_numeric_avx2(SB), $8-40
MOVQ itype+0(FP), DI
MOVQ otype+8(FP), SI
diff --git a/arrow/compute/internal/kernels/cast_numeric_sse4_amd64.s
b/arrow/compute/internal/kernels/cast_numeric_sse4_amd64.s
index 778118ff..bc74ee49 100644
--- a/arrow/compute/internal/kernels/cast_numeric_sse4_amd64.s
+++ b/arrow/compute/internal/kernels/cast_numeric_sse4_amd64.s
@@ -38,7 +38,7 @@ DATA LCDATA1<>+0x108(SB)/8, $0x0000000000000000
DATA LCDATA1<>+0x110(SB)/8, $0x000000005f000000
GLOBL LCDATA1<>(SB), 8, $280
-TEXT ·_cast_type_numeric_sse4(SB), $0-40
+TEXT ·_cast_type_numeric_sse4(SB), $8-40
MOVQ itype+0(FP), DI
MOVQ otype+8(FP), SI
