You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/08/25 13:23:37 UTC
[GitHub] [arrow] pitrou opened a new pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
pitrou opened a new pull request #8049:
URL: https://github.com/apache/arrow/pull/8049
Valgrind is still lacking support for AVX512 instructions:
https://bugs.kde.org/show_bug.cgi?id=383010
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680045478
[AMD64 Conda Crossbow Submit (#123814)](https://ci.ursalabs.org/#builders/98/builds/650) builder has been succeeded.
Revision: 18b0ce28b86b6444efcce5096612c1674137604f
Submitted crossbow builds: [ursa-labs/crossbow @ ursabot-579](https://github.com/ursa-labs/crossbow/branches/all?query=ursabot-579)
|Task|Status|
|----|------|
|test-conda-cpp-valgrind|[](https://github.com/ursa-labs/crossbow/actions?query=branch:ursabot-579-github-test-conda-cpp-valgrind)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680039551
[AMD64 Conda Crossbow Submit (#123805)](https://ci.ursalabs.org/#builders/98/builds/649) builder has been succeeded.
Revision: 8cc8c2b10fe963c296673e489cdb9f9080b6aa8b
Submitted crossbow builds: [ursa-labs/crossbow @ ursabot-578](https://github.com/ursa-labs/crossbow/branches/all?query=ursabot-578)
|Task|Status|
|----|------|
|test-conda-cpp-valgrind|[](https://github.com/ursa-labs/crossbow/actions?query=branch:ursabot-578-github-test-conda-cpp-valgrind)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680022432
@github-actions crossbow submit conda-cpp-valgrind
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jianxind edited a comment on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
jianxind edited a comment on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-681200942
I will create a JIRA for adding upper case to ARROW_USER_SIMD_LEVEL also. PR: https://github.com/apache/arrow/pull/8060
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680091518
@jianxind What is your opinion on the approach here?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-681945189
Since ARROW_USER_SIMD_LEVEL (environment variable) is enough to fix this, do we want to keep ARROW_RUNTIME_SIMD_LEVEL (CMake setting) or not?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680024980
[AMD64 Conda Crossbow Submit (#123788)](https://ci.ursalabs.org/#builders/98/builds/648) builder has been succeeded.
Revision: c2c7526152ef73b7b54949ecd266594181085454
Submitted crossbow builds: [ursa-labs/crossbow @ ursabot-577](https://github.com/ursa-labs/crossbow/branches/all?query=ursabot-577)
|Task|Status|
|----|------|
|test-conda-cpp-valgrind|[](https://github.com/ursa-labs/crossbow/actions?query=branch:ursabot-577-github-test-conda-cpp-valgrind)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#discussion_r477302093
##########
File path: cpp/src/arrow/util/decimal_test.cc
##########
@@ -384,10 +389,10 @@ static const ToStringTestData kToStringTestData[] = {
{-1234567890123456789LL, 25, "-1.234567890123456789E-7"},
};
-class Decimal128ToStringTest : public ::testing::TestWithParam<ToStringTestData> {};
+class Decimal128ToStringTest : public ::testing::TestWithParam<ToStringTestParam> {};
TEST_P(Decimal128ToStringTest, ToString) {
- const ToStringTestData& data = GetParam();
+ const ToStringTestParam& data = GetParam();
Review comment:
Done.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on a change in pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#discussion_r476921895
##########
File path: cpp/src/arrow/util/decimal_test.cc
##########
@@ -384,10 +389,10 @@ static const ToStringTestData kToStringTestData[] = {
{-1234567890123456789LL, 25, "-1.234567890123456789E-7"},
};
-class Decimal128ToStringTest : public ::testing::TestWithParam<ToStringTestData> {};
+class Decimal128ToStringTest : public ::testing::TestWithParam<ToStringTestParam> {};
TEST_P(Decimal128ToStringTest, ToString) {
- const ToStringTestData& data = GetParam();
+ const ToStringTestParam& data = GetParam();
Review comment:
How about changing local variable name to `param` from `data` too?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jianxind commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
jianxind commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680399431
> > Another potential approach is the ARROW_USER_SIMD_LEVEL flag during runtime, something like "ARROW_USER_SIMD_LEVEL=avx2 make unittest", I'm not familiar with CI system thus don't know if it's easy to hook ARROW_USER_SIMD_LEVEL flag.
>
> We can change environment variables only for test in CI.
>
> But I don't know whether Valgrind can work with binary that has AVX512 instructions or not...
Probably yes I think, as current approach only disable the AVX512 register path, the AVX512 file is still built into the lib.
The AVX512 file is defined under CXX_SUPPORTS_AVX512 flag
```
if(CXX_SUPPORTS_AVX512)
list(APPEND ARROW_SRCS util/bpacking_avx512.cc)
set_source_files_properties(util/bpacking_avx512.cc PROPERTIES SKIP_PRECOMPILE_HEADERS
ON)
set_source_files_properties(util/bpacking_avx512.cc PROPERTIES COMPILE_FLAGS
${ARROW_AVX512_FLAG})
endif()
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680423797
OK. Then I think that the environment variable approach is better because we already have `ARROW_USER_SIMD_LEVEL` support (I didn't know!). The following patch will work:
```diff
diff --git a/docker-compose.yml b/docker-compose.yml
index 86cffad81..55da4b60b 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -199,6 +199,7 @@ services:
ARROW_JEMALLOC: "OFF"
ARROW_S3: "OFF"
ARROW_TEST_MEMCHECK: "ON"
+ ARROW_USER_SIMD_LEVEL: "AVX2" # AVX512 not supported by Valgrind (ARROW-9851)
ARROW_USE_LD_GOLD: "ON"
BUILD_WARNING_LEVEL: "PRODUCTION"
volumes: *conda-volumes
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jianxind edited a comment on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
jianxind edited a comment on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680426063
> OK. Then I think that the environment variable approach is better because we already have `ARROW_USER_SIMD_LEVEL` support (I didn't know!). The following patch will work:
>
> ```diff
> diff --git a/docker-compose.yml b/docker-compose.yml
> index 86cffad81..55da4b60b 100644
> --- a/docker-compose.yml
> +++ b/docker-compose.yml
> @@ -199,6 +199,7 @@ services:
> ARROW_JEMALLOC: "OFF"
> ARROW_S3: "OFF"
> ARROW_TEST_MEMCHECK: "ON"
> + ARROW_USER_SIMD_LEVEL: "AVX2" # AVX512 not supported by Valgrind (ARROW-9851)
> ARROW_USE_LD_GOLD: "ON"
> BUILD_WARNING_LEVEL: "PRODUCTION"
> volumes: *conda-volumes
> ```
ARROW_USER_SIMD_LEVEL is a ENV variants, will this change pass AVX2 to ARROW_USER_SIMD_LEVEL env?
https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/cpu_info.cc#L423
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680030260
https://issues.apache.org/jira/browse/ARROW-9851
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jianxind commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
jianxind commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-681200942
I will create a JIRA for adding upper case to ARROW_USER_SIMD_LEVEL also.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680501869
> ARROW_USER_SIMD_LEVEL is a ENV variants, will this change pass AVX2 to ARROW_USER_SIMD_LEVEL env?
Yes. But the patch was wrong. We need to use lower case for `ARROW_USER_SIMD_LEVEL`:
```diff
diff --git a/docker-compose.yml b/docker-compose.yml
index 86cffad81..626292358 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -199,6 +199,7 @@ services:
ARROW_JEMALLOC: "OFF"
ARROW_S3: "OFF"
ARROW_TEST_MEMCHECK: "ON"
+ ARROW_USER_SIMD_LEVEL: "avx2" # AVX512 not supported by Valgrind (ARROW-9851)
ARROW_USE_LD_GOLD: "ON"
BUILD_WARNING_LEVEL: "PRODUCTION"
volumes: *conda-volumes
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680024383
@ursabot crossbow submit test-conda-cpp-valgrind
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-681096064
I agree with you including upper case.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-683163747
OK.
I merge this.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680090251
[AMD64 Conda Crossbow Submit (#123831)](https://ci.ursalabs.org/#builders/98/builds/651) builder has been succeeded.
Revision: 5461e01225db7eaca9484213c0169f8030a3e3ba
Submitted crossbow builds: [ursa-labs/crossbow @ ursabot-580](https://github.com/ursa-labs/crossbow/branches/all?query=ursabot-580)
|Task|Status|
|----|------|
|test-conda-cpp-valgrind|[](https://github.com/ursa-labs/crossbow/actions?query=branch:ursabot-580-github-test-conda-cpp-valgrind)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680022671
@crossbow submit conda-cpp-valgrind
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-681945538
@ursabot crossbow submit test-conda-cpp-valgrind
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680089365
@ursabot crossbow submit test-conda-cpp-valgrind
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680377715
@github-actions crossbow submit test-conda-cpp-valgrind
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680023886
[AMD64 Conda Crossbow Submit (#123787)](https://ci.ursalabs.org/#builders/98/builds/647) builder failed.
Revision: c2c7526152ef73b7b54949ecd266594181085454
Crossbow: `'python crossbow.py ...'` step's stderr:
```
Traceback (most recent call last):
File "crossbow.py", line 1645, in <module>
crossbow(obj={}, auto_envvar_prefix='CROSSBOW')
File "/opt/conda/lib/python3.8/site-packages/click/core.py", line 829, in __call__
return self.main(*args, **kwargs)
File "/opt/conda/lib/python3.8/site-packages/click/core.py", line 782, in main
rv = self.invoke(ctx)
File "/opt/conda/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/opt/conda/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/opt/conda/lib/python3.8/site-packages/click/core.py", line 610, in invoke
return callback(*args, **kwargs)
File "/opt/conda/lib/python3.8/site-packages/click/decorators.py", line 33, in new_func
return f(get_current_context().obj, *args, **kwargs)
File "crossbow.py", line 1444, in submit
job = Job.from_config(config=config, target=target, tasks=tasks,
File "crossbow.py", line 943, in from_config
return cls(target=target, tasks=tasks)
File "crossbow.py", line 868, in __init__
raise ValueError('no tasks were provided for the job')
ValueError: no tasks were provided for the job
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou removed a comment on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou removed a comment on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680022671
@crossbow submit conda-cpp-valgrind
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jianxind commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
jianxind commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680371722
> @jianxind What is your opinion on the approach here?
It's fine to introduce ARROW_RUNTIME_SIMD_LEVEL flag which give more user options. Another potential approach is the ARROW_USER_SIMD_LEVEL flag during runtime, something like "ARROW_USER_SIMD_LEVEL=avx2 make unittest", I'm not familiar with CI system thus don't know if it's easy to hook ARROW_USER_SIMD_LEVEL flag.
+1
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680382309
> Another potential approach is the ARROW_USER_SIMD_LEVEL flag during runtime, something like "ARROW_USER_SIMD_LEVEL=avx2 make unittest", I'm not familiar with CI system thus don't know if it's easy to hook ARROW_USER_SIMD_LEVEL flag.
We can change environment variables only for test in CI.
But I don't know whether Valgrind can work with binary that has AVX512 instructions or not...
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680035192
@ursabot crossbow submit test-conda-cpp-valgrind
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jianxind commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
jianxind commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-682263721
> Since ARROW_USER_SIMD_LEVEL (environment variable) is enough to fix this, do we want to keep ARROW_RUNTIME_SIMD_LEVEL (CMake setting) or not?
I will vote +1 to keep ARROW_RUNTIME_SIMD_LEVEL, a SIMD runtime level option at build time should benefit some cases.
For example, avx512 is disabled for MINGW build with a HACK. ARROW_RUNTIME_SIMD_LEVEL can be used here.
https://github.com/apache/arrow/blob/master/cpp/cmake_modules/SetupCxxFlags.cmake#L55
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jianxind commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
jianxind commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680426063
> OK. Then I think that the environment variable approach is better because we already have `ARROW_USER_SIMD_LEVEL` support (I didn't know!). The following patch will work:
>
> ```diff
> diff --git a/docker-compose.yml b/docker-compose.yml
> index 86cffad81..55da4b60b 100644
> --- a/docker-compose.yml
> +++ b/docker-compose.yml
> @@ -199,6 +199,7 @@ services:
> ARROW_JEMALLOC: "OFF"
> ARROW_S3: "OFF"
> ARROW_TEST_MEMCHECK: "ON"
> + ARROW_USER_SIMD_LEVEL: "AVX2" # AVX512 not supported by Valgrind (ARROW-9851)
> ARROW_USE_LD_GOLD: "ON"
> BUILD_WARNING_LEVEL: "PRODUCTION"
> volumes: *conda-volumes
> ```
https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/cpu_info.cc#L423
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680380195
Revision: 5461e01225db7eaca9484213c0169f8030a3e3ba
Submitted crossbow builds: [ursa-labs/crossbow @ actions-499](https://github.com/ursa-labs/crossbow/branches/all?query=actions-499)
|Task|Status|
|----|------|
|test-conda-cpp-valgrind|[](https://github.com/ursa-labs/crossbow/actions?query=branch:actions-499-github-test-conda-cpp-valgrind)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680880304
> We need to use lower case for ARROW_USER_SIMD_LEVEL
Hmm, that's a pity. Can't we use the same convention everywhere?
(I would probably favour uppercase, since those names are generally abbreviations)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680022894
@ursabot crossbow submit conda-cpp-valgrind
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-681948156
[AMD64 Conda Crossbow Submit (#124163)](https://ci.ursalabs.org/#builders/98/builds/652) builder has been succeeded.
Revision: 94a8b776cea1e75d431e7d6ae38d2e4b830593dc
Submitted crossbow builds: [ursa-labs/crossbow @ ursabot-581](https://github.com/ursa-labs/crossbow/branches/all?query=ursabot-581)
|Task|Status|
|----|------|
|test-conda-cpp-valgrind|[](https://github.com/ursa-labs/crossbow/actions?query=branch:ursabot-581-github-test-conda-cpp-valgrind)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou closed pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
kou closed pull request #8049:
URL: https://github.com/apache/arrow/pull/8049
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8049: ARROW-9851: [C++] Disable AVX512 runtime paths with Valgrind
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8049:
URL: https://github.com/apache/arrow/pull/8049#issuecomment-680044371
@ursabot crossbow submit test-conda-cpp-valgrind
----------------------------------------------------------------
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:
users@infra.apache.org