AntoinePrv commented on code in PR #50030:
URL: https://github.com/apache/arrow/pull/50030#discussion_r3316064749
##########
cpp/src/parquet/bloom_filter.cc:
##########
@@ -34,6 +35,35 @@
#include "parquet/thrift_internal.h"
#include "parquet/xxhasher.h"
+#if defined(ARROW_HAVE_RUNTIME_AVX2)
+# include "parquet/bloom_filter_avx2_internal.h"
+#endif
+
+#define PARQUET_IMPL_NAMESPACE standard
+#include "parquet/bloom_filter_block_inc.h"
+#undef PARQUET_IMPL_NAMESPACE
+
+#include "arrow/util/dispatch_internal.h"
+
+namespace parquet::internal {
+namespace {
+
+using ::arrow::internal::DynamicDispatch;
+
+struct FindHashBlockDynamicFunction {
+ using FunctionType = decltype(&standard::FindHashBlockImpl);
+
+ static constexpr auto targets() {
+ return std::array{
+ ARROW_DISPATCH_TARGET_NONE(&standard::FindHashBlockImpl) //
+ ARROW_DISPATCH_TARGET_AVX2(&FindHashBlockAvx2) //
Review Comment:
> ARROW_DISPATCH_TARGET_AVX2 expands when either ARROW_HAVE_RUNTIME_AVX2 or
ARROW_HAVE_AVX2 is defined (see arrow/util/dispatch_internal.h).
This yes.
> In a build with an AVX2 baseline (-DARROW_HAVE_AVX2) but with runtime
dispatch disabled (ARROW_HAVE_RUNTIME_AVX2 unset), this targets list will still
try to reference FindHashBlockAvx2
This too.
> even though bloom_filter_avx2_internal.h isn’t included and
bloom_filter_avx2.cc isn’t built
I thought in practice CMake will force `ARROW_HAVE_RUNTIME_AVX2` with
`ARROW_HAVE_AVX2`, but I cannot find any code to support this claim. If so,
then this is a problem general to Arrow.
Regardless of the build tool, a defensive pattern `defined(ARROW_HAVE_AVX2)
|| defined(ARROW_HAVE_RUNTIME_AVX2)` would be welcome.
--
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]
