[GitHub] [arrow] cyb70289 commented on a diff in pull request #37785: GH-37712: [Go][Parquet] Fix ARM64 assembly for bitmap extract bits

["cyb70289 (via GitHub)" <gi...@apache.org>]

cyb70289 commented on code in PR #37785:
URL: https://github.com/apache/arrow/pull/37785#discussion_r1332397446


##########
go/parquet/internal/bmi/bitmap_neon_arm64.s:
##########
@@ -27,8 +27,8 @@ LBB0_2:
     MOVD R0, res+16(FP)
     RET
 LBB0_4:
-    WORD $0xaa1f03e0 // mov    x0, xzr
     WORD $0xa8c17bfd // ldp    x29, x30, [sp], #16
+    MOVD ZR, res+16(FP)

Review Comment:
   > The assembly wasn't _generated_ by hand, but this fix was absolutely written by hand (me) by way of looking at the code, the generated assembly, and stepping through the instructions to figure out what was happening incorrectly.
   > 
   
   This is cool!
   
   Might be bug from asm2plan9s. Or errors from manually converting/formatting from normal arm assembly to plan9.
   Not sure the best approach to convert normal C/assembly to plan9 assembly. [asm2plan9s](https://github.com/minio/asm2plan9s) and [c2goasm](https://github.com/minio/c2goasm) are archived. Looks we should avoid them.
   
   For this pext like function, the go version (lookup table) should perform better than this one. Maybe we can just delete this assembly implementation?



-- 
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