zeroshade commented on a change in pull request #9671: URL: https://github.com/apache/arrow/pull/9671#discussion_r595168202
########## File path: go/parquet/internal/bmi/_lib/bitmap_bmi2.c ########## @@ -0,0 +1,38 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include <stdint.h> +#include <x86intrin.h> + +void extract_bits(uint64_t bitmap, uint64_t select_bitmap, uint64_t* res) { + *res = _pext_u64(bitmap, select_bitmap); +} + +void popcount64(uint64_t bitmap, uint64_t* res) { Review comment: 1. Sure, I'll go through and add more comments and docs to various functions and areas. 2. Since Go is statically linked and compiled and pulled via `go get` which will not automatically call `go generate` for you (go explicitly avoids doing things when it isn't explicitly requested to do so) the pattern in Go development currently requires checking in all generated code. The assembly in the _lib dirs technically isn't necessary to be checked in as they are just an intermediate step to the plan 9 assembly in the actual package directories, but I was following the pattern set in the go arrow package which checked in that assembly also. This is why I included the Makefiles which are used to generate the ASM files so they can be easily regenerated if needed. 3. This is something i was considering also, there's a `bitutil` package in the Go arrow module and was considering putting many of these utilities there. For now, I was taking advantage of the special "internal" path for packages in Go in that anything under an `internal` directory is not able to be imported outside of the module, so right now all of these utilities are not importable outside of the `parquet` module itself. If you think it's worthwhile, I have no problem shifting them to the `bitutil` package in the Arrow go module though. The only reason I didn't start with that was because I liked the idea of ensuring they aren't currently exported outside of this package for now. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
