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/05 07:55:25 UTC

[GitHub] [arrow] jianxind opened a new pull request #7903: ARROW-9643: [C++] only call the SIMD register when it's suppored.

jianxind opened a new pull request #7903:
URL: https://github.com/apache/arrow/pull/7903


   Compiler may use advanced instruction just for a register routine.
   
   Signed-off-by: Frank Du <fr...@intel.com>


----------------------------------------------------------------
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 a change in pull request #7903: ARROW-9643: [C++] Only register the SIMD variants when it's supported.

Posted by GitBox <gi...@apache.org>.
jianxind commented on a change in pull request #7903:
URL: https://github.com/apache/arrow/pull/7903#discussion_r465600581



##########
File path: cpp/src/arrow/compute/kernels/aggregate_basic.cc
##########
@@ -396,11 +397,16 @@ void RegisterScalarAggregateBasic(FunctionRegistry* registry) {
   aggregate::AddBasicAggKernels(aggregate::SumInit, FloatingPointTypes(), float64(),
                                 func.get());
   // Add the SIMD variants for sum
+  auto cpu_info = arrow::internal::CpuInfo::GetInstance();
 #if defined(ARROW_HAVE_RUNTIME_AVX2)

Review comment:
       No. Other arch build like ARM/PPC will not have AddSumAvx2AggKernels function.




----------------------------------------------------------------
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 #7903: ARROW-9643: [C++] only call the SIMD register when it's suppored.

Posted by GitBox <gi...@apache.org>.
jianxind commented on pull request #7903:
URL: https://github.com/apache/arrow/pull/7903#issuecomment-669045010


   cc @cyb70289 @wesm 


----------------------------------------------------------------
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] cyb70289 commented on a change in pull request #7903: ARROW-9643: [C++] Only register the SIMD variants when it's supported.

Posted by GitBox <gi...@apache.org>.
cyb70289 commented on a change in pull request #7903:
URL: https://github.com/apache/arrow/pull/7903#discussion_r465597132



##########
File path: cpp/src/arrow/compute/kernels/aggregate_basic.cc
##########
@@ -396,11 +397,16 @@ void RegisterScalarAggregateBasic(FunctionRegistry* registry) {
   aggregate::AddBasicAggKernels(aggregate::SumInit, FloatingPointTypes(), float64(),
                                 func.get());
   // Add the SIMD variants for sum
+  auto cpu_info = arrow::internal::CpuInfo::GetInstance();
 #if defined(ARROW_HAVE_RUNTIME_AVX2)

Review comment:
       Is it possible to drop these #ifdef ?




----------------------------------------------------------------
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] nealrichardson commented on pull request #7903: ARROW-9643: [C++] Only register the SIMD variants when it's supported.

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #7903:
URL: https://github.com/apache/arrow/pull/7903#issuecomment-670564397


   @github-actions crossbow submit -g r


----------------------------------------------------------------
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 #7903: ARROW-9643: [C++] Only register the SIMD variants when it's supported.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7903:
URL: https://github.com/apache/arrow/pull/7903#issuecomment-669047146


   https://issues.apache.org/jira/browse/ARROW-9643


----------------------------------------------------------------
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 #7903: ARROW-9643: [C++] Only register the SIMD variants when it's supported.

Posted by GitBox <gi...@apache.org>.
jianxind commented on pull request #7903:
URL: https://github.com/apache/arrow/pull/7903#issuecomment-670254114


   Ping, @wesm @pitrou @emkornfield 
   
   Sorry to bother you, but can we prioritize this fix as it's a critical panic bug? The fix is simple enough.


----------------------------------------------------------------
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] wesm closed pull request #7903: ARROW-9643: [C++] Only register the SIMD variants when it's supported.

Posted by GitBox <gi...@apache.org>.
wesm closed pull request #7903:
URL: https://github.com/apache/arrow/pull/7903


   


----------------------------------------------------------------
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 #7903: ARROW-9643: [C++] Only register the SIMD variants when it's supported.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7903:
URL: https://github.com/apache/arrow/pull/7903#issuecomment-670565444


   Revision: 4300febdbf53bfebc98e5cea88b4b065ffc22041
   
   Submitted crossbow builds: [ursa-labs/crossbow @ actions-469](https://github.com/ursa-labs/crossbow/branches/all?query=actions-469)
   
   |Task|Status|
   |----|------|
   |homebrew-r-autobrew|[![TravisCI](https://img.shields.io/travis/ursa-labs/crossbow/actions-469-travis-homebrew-r-autobrew.svg)](https://travis-ci.org/ursa-labs/crossbow/branches)|
   |test-conda-r-4.0|[![Github Actions](https://github.com/ursa-labs/crossbow/workflows/Crossbow/badge.svg?branch=actions-469-github-test-conda-r-4.0)](https://github.com/ursa-labs/crossbow/actions?query=branch:actions-469-github-test-conda-r-4.0)|
   |test-r-linux-as-cran|[![Github Actions](https://github.com/ursa-labs/crossbow/workflows/Crossbow/badge.svg?branch=actions-469-github-test-r-linux-as-cran)](https://github.com/ursa-labs/crossbow/actions?query=branch:actions-469-github-test-r-linux-as-cran)|
   |test-r-rhub-ubuntu-gcc-release|[![Azure](https://dev.azure.com/ursa-labs/crossbow/_apis/build/status/ursa-labs.crossbow?branchName=actions-469-azure-test-r-rhub-ubuntu-gcc-release)](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-469-azure-test-r-rhub-ubuntu-gcc-release)|
   |test-r-rocker-r-base-latest|[![Azure](https://dev.azure.com/ursa-labs/crossbow/_apis/build/status/ursa-labs.crossbow?branchName=actions-469-azure-test-r-rocker-r-base-latest)](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-469-azure-test-r-rocker-r-base-latest)|
   |test-r-rstudio-r-base-3.6-bionic|[![Azure](https://dev.azure.com/ursa-labs/crossbow/_apis/build/status/ursa-labs.crossbow?branchName=actions-469-azure-test-r-rstudio-r-base-3.6-bionic)](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-469-azure-test-r-rstudio-r-base-3.6-bionic)|
   |test-r-rstudio-r-base-3.6-centos6|[![Azure](https://dev.azure.com/ursa-labs/crossbow/_apis/build/status/ursa-labs.crossbow?branchName=actions-469-azure-test-r-rstudio-r-base-3.6-centos6)](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-469-azure-test-r-rstudio-r-base-3.6-centos6)|
   |test-r-rstudio-r-base-3.6-centos8|[![Azure](https://dev.azure.com/ursa-labs/crossbow/_apis/build/status/ursa-labs.crossbow?branchName=actions-469-azure-test-r-rstudio-r-base-3.6-centos8)](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-469-azure-test-r-rstudio-r-base-3.6-centos8)|
   |test-r-rstudio-r-base-3.6-opensuse15|[![Azure](https://dev.azure.com/ursa-labs/crossbow/_apis/build/status/ursa-labs.crossbow?branchName=actions-469-azure-test-r-rstudio-r-base-3.6-opensuse15)](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-469-azure-test-r-rstudio-r-base-3.6-opensuse15)|
   |test-r-rstudio-r-base-3.6-opensuse42|[![Azure](https://dev.azure.com/ursa-labs/crossbow/_apis/build/status/ursa-labs.crossbow?branchName=actions-469-azure-test-r-rstudio-r-base-3.6-opensuse42)](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-469-azure-test-r-rstudio-r-base-3.6-opensuse42)|
   |test-ubuntu-18.04-r-sanitizer|[![Azure](https://dev.azure.com/ursa-labs/crossbow/_apis/build/status/ursa-labs.crossbow?branchName=actions-469-azure-test-ubuntu-18.04-r-sanitizer)](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-469-azure-test-ubuntu-18.04-r-sanitizer)|


----------------------------------------------------------------
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] cyb70289 commented on pull request #7903: ARROW-9643: [C++] Only register the SIMD variants when it's supported.

Posted by GitBox <gi...@apache.org>.
cyb70289 commented on pull request #7903:
URL: https://github.com/apache/arrow/pull/7903#issuecomment-669078062


   Validated okay on Haswell


----------------------------------------------------------------
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