You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "nsrip-dd (via GitHub)" <gi...@apache.org> on 2026/07/22 18:32:11 UTC
[I] arm64 assembly breaks CPU profiling [arrow-go]
nsrip-dd opened a new issue, #983:
URL: https://github.com/apache/arrow-go/issues/983
### Describe the bug, including details regarding any error messages, version, and platform.
The arm64 NEON assembly implementations of functions in this library break CPU profiling. This breakage usually manifests as truncated and sometimes invalid tracebacks in the profile and sometimes crashes. The reason for this is that the functions tend to manipulate the stack pointer, but via `WORD` directives rather than normal instructions, e.g. `WORD $0xa9ba7bfd // stp x29, x30, [sp, #-96]!`. The Go assembler doesn't decode these instructions and thus doesn't see the stack pointer manipulation. But this means that the generated unwinding tables will be invalid. The tables encode how much to increase the stack pointer at a given instruction to find the next call frame.
## Examples
For example, `go test -bench=. -cpuprofile=cpu.pprof ./arrow/memory` produces a profile like
<img width="886" height="371" alt="Image" src="https://github.com/user-attachments/assets/af53b513-7919-40f4-bf12-940c32fa77dc" />
Notice that most of the `memory._memset_neon` time is in a traceback with one frame, missing the callers.
For another example, consider this benchmark of `parquet/internal/utils`:
```
func BenchmarkUnpack32(b *testing.B) {
const batchSize = 512
input := make([]byte, batchSize*4)
if _, err := rand.Read(input); err != nil {
b.Fatal(err)
}
output := make([]uint64, batchSize)
reader := NewBitReader(bytes.NewReader(input))
for b.Loop() {
reader.Reset(bytes.NewReader(input))
reader.GetBatch(32, output)
}
}
```
The CPU time attributed to `_unpack32_neon`, which gets called here, looks like this in the CPU profile:
<img width="886" height="181" alt="Image" src="https://github.com/user-attachments/assets/f15af53f-17eb-4bc0-813a-a09d43df26ca" />
There are either no callers, or an invalid call sequence showing the runtime as the caller of this function.
The same benchmark actually crashes the CPU profiler when with cgo disabled, i.e. `CGO_ENABLED=0 go test -bench=BenchmarkUnpack32 ./parquet/internal/utils`. This is because `_unpack32_neon` writes to register R28/x28, which is reserved by the runtime to hold the current goroutine address. [Ref](https://cs.opensource.google/go/go/+/master:src/cmd/compile/abi-internal.md). With cgo disabled, the CPU profiling signal handler reads the goroutine directly from this register, crashing because it is invalid. The benchmark was constructed to trigger this case.
## Suggested fixes
- Get rid of any instructions that write to the stack pointer.
- For functions which need a stack frame, give them a stack frame by modifying the assembly function declaration. For example, `TEXT ·_unpack32_neon(SB), $0-40` would become `TEXT ·_unpack32_neon(SB), $496-40` because it needs a 496 byte frame. (I think it could even be 480, but my current draft fix had accounted for )
- Regenerate/modify `_unpack32_neon` to avoid using registers R28 and R18, which are reserved.
I can send a PR for the first two fixes. I haven't tried the third fix yet and it might be a bit more involved. An ideal long term fix would be to port all the `WORD` directives to actual instructions but that's 1) a big change and 2) blocked by touching R28 and R18, which the assembler normally doesn't allow.
NB: I think these bugs _might_ cause crashes like in https://github.com/golang/go/issues/62086, where unwinding gets stuck, but I haven't reproduced that specific failure mode yet.
### Component(s)
Other
--
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: issues-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] arm64 assembly breaks CPU profiling [arrow-go]
Posted by "nsrip-dd (via GitHub)" <gi...@apache.org>.
nsrip-dd commented on issue #983:
URL: https://github.com/apache/arrow-go/issues/983#issuecomment-5332134569
Regarding `_unpack32_neon`, I don't know if I'll have bandwidth any time soon to work on this, but I wanted to drop some breadcrumbs here in case anybody else wants to pick it up.
My high-level thought is that the best approach would be to re-generate the assembly without using the reserved registers, then re-apply any fixups/manual changes that have been done since the code was first generated. I had an LLM take a shot at fixing the assembly in-place but I have low confidence in the correctness.
We'd start by telling the C compiler not to use the reserved registers:
```diff
diff --git a/parquet/internal/utils/Makefile b/parquet/internal/utils/Makefile
index f6dce46197..dda1662c6b 100644
--- a/parquet/internal/utils/Makefile
+++ b/parquet/internal/utils/Makefile
@@ -27,7 +27,8 @@
ASM_FLAGS_POPCNT=-mpopcnt
C_FLAGS_NEON=-O3 -fvectorize -mllvm -force-vector-width=16 -fno-asynchronous-unwind-tables -mno-red-zone -mstackrealign -fno-exceptions \
- -fno-rtti -fno-builtin -ffast-math -fno-jump-tables -I_lib
+ -fno-rtti -fno-builtin -ffast-math -fno-jump-tables -I_lib \
+ -ffixed-x28 -ffixed-x27 -ffixed-x18
GO_SOURCES := $(shell find . -path ./_lib -prune -o -name '*.go' -not -name '*_test.go')
ALL_SOURCES := $(shell find . -path ./_lib -prune -o -name '*.go' -name '*.s' -not -name '*_test.go')
```
Then `make CC=clang _lib/bit_packing_neon.s` (I don't have clang-11 which is the default CC value; do we care about the specific clang version?). That'll give you the assembly from which we can build the Go assembly following the instructions in the parquet/internal/utils/_lib README file. Then we'd need to remove the frame pointer setup stuff, at the very least.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] arm64 assembly breaks CPU profiling [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on issue #983:
URL: https://github.com/apache/arrow-go/issues/983#issuecomment-5049988448
Interesting catch! Please send a PR for the first two fixes, afterwards we can look into the third fix as a follow-up
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] arm64 assembly breaks CPU profiling [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on issue #983:
URL: https://github.com/apache/arrow-go/issues/983#issuecomment-5589253405
@nsrip-dd picked up your breadcrumbs in #1300 — thanks, the `-ffixed` approach worked out even better than expected. With `-ffixed-x18 -ffixed-x27 -ffixed-x28` plus `-fomit-frame-pointer` (and without `-mstackrealign`, which forces a frame), clang 21 emits `unpack32_neon` as a leaf with zero stack usage, so there is no frame setup left to strip at all: constants go through `VMOVQ` pseudo-instructions and every branch is a real Go branch, leaving nothing SP-related hidden in `WORD`s.
Rather than the c2goasm path, the PR adds a small `_lib/neon2goasm.py` that converts the compiled object directly and hard-rejects any instruction touching SP or a reserved register, wired to `make bit_packing_neon_arm64.s`. I verified instruction-for-instruction equivalence against the ELF object, your `BenchmarkUnpack32` repro now profiles with full caller chains and no longer crashes under `CGO_ENABLED=0`, and throughput improved ~27% from the newer codegen.
`unpack_bool_neon_arm64.s` still has one `WORD`-encoded `stp` with SP writeback — same pattern, much smaller; follow-up material.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org