zeroshade opened a new pull request, #1432: URL: https://github.com/apache/arrow-go/pull/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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
