zeroshade commented on code in PR #1348:
URL: https://github.com/apache/arrow-go/pull/1348#discussion_r4189304922
##########
arrow/compute/internal/kernels/get_take_indices_avx2_amd64.s:
##########
@@ -0,0 +1,135 @@
+//go:build go1.18 && amd64 && !noasm && !appengine
+// AUTO-GENERATED BY C2GOASM -- DO NOT EDIT
+
+DATA LCDATA1<>+0x000(SB)/8, $0x0000000100000000
+DATA LCDATA1<>+0x008(SB)/8, $0x0000000300000002
+DATA LCDATA1<>+0x010(SB)/8, $0x0000000500000004
+DATA LCDATA1<>+0x018(SB)/8, $0x0000000700000006
+DATA LCDATA1<>+0x020(SB)/8, $0x0000000000000008
+GLOBL LCDATA1<>(SB), 8, $40
+
+TEXT ยท_get_take_indices_uint32_avx2(SB), $0-40
+
+ MOVQ filter+0(FP), DI
+ MOVQ output+8(FP), SI
+ MOVQ tables+16(FP), DX
+ MOVQ nbytes+24(FP), CX
+ MOVQ tailMask+32(FP), R8
+ LEAQ LCDATA1<>(SB), BP
Review Comment:
`LEAQ LCDATA1<>(SB), BP` overwrites the caller's frame pointer and nothing
restores it. This is what crashes `runtime.fpTracebackPCs` under the execution
tracer.
##########
arrow/compute/internal/kernels/Makefile:
##########
@@ -76,6 +76,9 @@ _lib/scalar_comparison_sse4_amd64.s: _lib/scalar_comparison.cc
_lib/filter_uint32_avx2_amd64.s: _lib/filter_uint32.cc
$(CXX) -std=c++17 -S $(C_FLAGS) $(ASM_FLAGS_AVX2) $^ -o $@ ;
$(PERL_FIXUP_ROTATE) $@
+_lib/get_take_indices_avx2_amd64.s: _lib/get_take_indices_avx2_amd64.cc
+ $(CXX) -std=c++17 -S $(C_FLAGS) $(ASM_FLAGS_AVX2) $^ -o $@ ;
$(PERL_FIXUP_ROTATE) $@
Review Comment:
This recipe copies the `filter_uint32` flags. Please add `-mno-stackrealign
-fomit-frame-pointer`, as the `filter_uint64` rule does, and move the constants
out of the constant pool so c2goasm doesn't need BP.
--
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]
