Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++] Arrow io memory_benchmark does not compile if SIMD level is NEON #32845

Closed
asfimport opened this issue Sep 1, 2022 · 4 comments
Closed

Comments

@asfimport
Copy link
Collaborator

When compiling Arrow with the following parameter:

-DARROW_BUILD_BENCHMARKS_REFERENCE="ON"

I get this error and other similar ones:

cpp/src/arrow/io/memory_benchmark.cc:244:26: error: use of undeclared identifier 'Read'; did you mean 'read'?
using ApplyFn = decltype(Read);

 

The reason is that memory_benchmark.cc does not define the Read function, and others, if the SIMD level is NEON (ARROW_HAVE_NEON is true).

Reporter: Aldrin Montana / @drin
Assignee: Aldrin Montana / @drin

PRs and other links:

Note: This issue was originally created as ARROW-17598. Please see the migration documentation for further details.

@asfimport
Copy link
Collaborator Author

Aldrin Montana / @drin:
I think the easiest thing to do would be to change cmake to not add the benchmark if the SIMD level is NEON:

diff --git a/cpp/src/arrow/io/CMakeLists.txt b/cpp/src/arrow/io/CMakeLists.txt
index 33de37c84..ddf10078b 100644
--- a/cpp/src/arrow/io/CMakeLists.txt
+++ b/cpp/src/arrow/io/CMakeLists.txt
@@ -36,7 +36,8 @@ add_arrow_test(memory_test PREFIX "arrow-io") add_arrow_benchmark(file_benchmark PREFIX "arrow-io")-if(NOT (${ARROW_SIMD_LEVEL} STREQUAL "NONE"))
+if(NOT (${ARROW_SIMD_LEVEL} STREQUAL "NONE") AND
+   NOT (${ARROW_SIMD_LEVEL} STREQUAL "NEON"))
   # This benchmark either requires SSE4.2 or ARMV8 SIMD to be enabled
   add_arrow_benchmark(memory_benchmark PREFIX "arrow-io")
 endif()

@asfimport
Copy link
Collaborator Author

Yibo Cai / @cyb70289:
Thanks @drin, will you open a PR?

@asfimport
Copy link
Collaborator Author

Aldrin Montana / @drin:
sounds good @cyb70289. I wasn't sure if it would need any more discussion (which is why I made a JIRA instead of submitted a MINOR pr).

I opened a PR, but I don't think the CI will exercise it, so not sure what to do about that. Let me know if you'd like any other follow up!

@asfimport
Copy link
Collaborator Author

Yibo Cai / @cyb70289:
Issue resolved by pull request 14036
#14036

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant