zeroshade opened a new pull request, #1300: URL: https://github.com/apache/arrow-go/pull/1300
### Rationale for this change Addresses the `_unpack32_neon` part of #983 following the regeneration approach outlined in https://github.com/apache/arrow-go/issues/983#issuecomment-5332134569. The previous assembly repurposed Go's reserved registers (R18 platform, R27 REGTMP, R28 `g`) and adjusted SP through opaque `WORD` directives, so the assembler's pcsp tables were wrong and `sigprof` could observe a clobbered `g` — truncated/invalid tracebacks, and a crash with `CGO_ENABLED=0`. ### What changes are included in this PR? - Regenerate `bit_packing_neon_arm64.s` from `_lib/bit_packing_neon.c` with `-ffixed-x18 -ffixed-x27 -ffixed-x28 -fomit-frame-pointer` (per the issue comment, plus omitting the frame). The result is a **leaf function with zero stack usage and zero reserved-register usage**: constants load via `VMOVQ` pseudo-instructions and every branch is a real Go branch instruction, so unwind state is always valid. - Add `_lib/neon2goasm.py`, which converts the compiled ELF object into the Go assembly and **rejects any instruction touching SP or a reserved register**, plus a `make bit_packing_neon_arm64.s` rule (`C_FLAGS_NEON_GOASM`) so regeneration is reproducible. - Drop the unused `<string.h>` include so the C compiles freestanding for cross-generation. - Add `BenchmarkUnpack32` (from the issue) so the packed-decode fast path is exercised under `-cpuprofile`. - Document the new generation path in `_lib/README.md`. ### Verification - Semantic equivalence: instruction-by-instruction comparison of the assembled Go function against the source ELF object (branch topology index-normalized, `VMOVQ` constants resolved and compared against `.rodata`) — no differences besides the parameter/return glue. - `go test ./parquet/internal/utils` (native NEON), `-race`, `-tags noasm`, and `go test -tags assert ./parquet/...` all pass on Apple Silicon. - The issue's failure mode is fixed: `CGO_ENABLED=0 go test -bench BenchmarkUnpack32 -cpuprofile` no longer crashes, and profiles now show full caller chains: ```text 10ms utils._unpack32_neon utils.unpack32NEON utils.(*BitReader).GetBatch utils.BenchmarkUnpack32 testing.(*B).runN ``` - Throughput improved ~27% as a side effect of the newer codegen (387 → 280 ns/op on `BenchmarkUnpack32`, Apple M4). ### Are there any user-facing changes? No API changes. CPU profiles of workloads that hit the ARM64 packed decoder are now correct. Note: `unpack_bool_neon_arm64.s` has a smaller instance of the same pattern (one `WORD`-encoded `stp` with SP writeback); left for a follow-up to keep this reviewable. -- 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]
