edponce commented on a change in pull request #11882:
URL: https://github.com/apache/arrow/pull/11882#discussion_r785333007
##########
File path: cpp/src/arrow/util/bit_block_counter.h
##########
@@ -46,7 +46,7 @@ inline uint64_t ShiftWord(uint64_t current, uint64_t next,
int64_t shift) {
}
// These templates are here to help with unit tests
-
+// Two Arguments
template <typename T>
constexpr T BitNot(T x) {
Review comment:
This comment only applies to `BitBlockAnd` and `BitBlockAndNot`. IMO, I
would remove these comments because the function's signature if clear enough to
identify the number of arguments.
##########
File path: go/arrow/memory/_lib/arch.h
##########
@@ -22,8 +22,6 @@
#define FULL_NAME(x) x##_sse4
#elif __SSE3__ == 1
#define FULL_NAME(x) x##_sse3
-#elif defined(__ARM_NEON) || defined(__ARM_NEON__)
- #define FULL_NAME(x) x##_neon
#else
Review comment:
Why this change?
##########
File path: dev/tasks/java-jars/README.md
##########
@@ -16,7 +16,7 @@ See the License for the specific language governing
permissions and
limitations under the License.
-->
-# Java Jars Task
+# Jars.
Review comment:
Remove the period?
##########
File path: go/arrow/memory/_lib/arch.h
##########
@@ -22,8 +22,6 @@
#define FULL_NAME(x) x##_sse4
#elif __SSE3__ == 1
#define FULL_NAME(x) x##_sse3
-#elif defined(__ARM_NEON) || defined(__ARM_NEON__)
- #define FULL_NAME(x) x##_neon
#else
#define FULL_NAME(x) x##_x86
#endif
Review comment:
Missing EOF
##########
File path: go/arrow/memory/Makefile
##########
@@ -56,11 +46,9 @@ _lib/memory_avx2.s: _lib/memory.c
_lib/memory_sse4.s: _lib/memory.c
$(CC) -S $(C_FLAGS) $(ASM_FLAGS_SSE4) $^ -o $@ ; $(PERL_FIXUP_ROTATE) $@
-_lib/memory_neon.s: _lib/memory.c
- $(CC) -S $(C_FLAGS_NEON) $^ -o $@ ; $(PERL_FIXUP_ROTATE) $@
-
memory_avx2_amd64.s: _lib/memory_avx2.s
Review comment:
Why all these changes to this Makefile?
##########
File path: ci/docker/java-jni-manylinux-201x.dockerfile
##########
@@ -30,8 +30,7 @@ RUN vcpkg install --clean-after-build \
boost-regex \
boost-system \
boost-variant \
- # Use enable rtti to avoid link problems in Gandiva
- llvm[clang,default-options,default-targets,lld,tools,enable-rtti]
+ llvm
Review comment:
Why this change?
--
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]
