You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "amoeba (via GitHub)" <gi...@apache.org> on 2024/03/28 03:37:14 UTC

[PR] GH-40806: [C++] Correctly report asimd/neon in GetRuntimeInfo [arrow]

amoeba opened a new pull request, #40857:
URL: https://github.com/apache/arrow/pull/40857

   ### What changes are included in this PR?
   
   New case to conditional in `MakeSimdLevelString` which makes `GetRuntimeInfo` report correctly on respective CPUs. I chose to have it report "neon". Lowercase to match other strings and "neon" instead of "asimd" because I think that makes more sense to users. I'm not 100% sure which is more correct.
   
   Fixes #40806
   
   ### Are these changes tested?
   
   We don't have automated tests for this. I did install the R package and, on my M1 laptop it reports 'neon' now instead of 'none' before:
   
   ```r
   > arrow_info()
   ...
   SIMD Level          neon
   Detected SIMD Level neon
   ```
   
   ### Are there any user-facing changes?
   
   No.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-40806: [C++] Correctly report asimd/neon in GetRuntimeInfo [arrow]

Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on PR #40857:
URL: https://github.com/apache/arrow/pull/40857#issuecomment-2035196813

   Thanks for catching this @pitrou.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-40806: [C++] Correctly report asimd/neon in GetRuntimeInfo [arrow]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #40857:
URL: https://github.com/apache/arrow/pull/40857#issuecomment-2033794599

   Hello,
   There are two problems with this PR:
   1. the possible values should reflect the allowed values for `ARROW_USER_SIMD_LEVEL` (which "neon" is not part of)
   2. it broke the [arm64 wheel tests](https://github.com/ursacomputing/crossbow/actions/runs/8526875367/job/23357055648#step:10:791):
   ```
   =================================== FAILURES ===================================
   ______________________________ test_runtime_info _______________________________
   
       def test_runtime_info():
           info = pa.runtime_info()
           assert isinstance(info, pa.RuntimeInfo)
           possible_simd_levels = ('none', 'sse4_2', 'avx', 'avx2', 'avx512')
   >       assert info.simd_level in possible_simd_levels
   E       AssertionError: assert 'neon' in ('none', 'sse4_2', 'avx', 'avx2', 'avx512')
   E        +  where 'neon' = RuntimeInfo(simd_level='neon', detected_simd_level='neon').simd_level
   ```
   
   Can this perhaps be reverted?


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-40806: [C++] Correctly report asimd/neon in GetRuntimeInfo [arrow]

Posted by "cyb70289 (via GitHub)" <gi...@apache.org>.
cyb70289 commented on PR #40857:
URL: https://github.com/apache/arrow/pull/40857#issuecomment-2034664878

   oops, then it should be reverted


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-40806: [C++] Correctly report asimd/neon in GetRuntimeInfo [arrow]

Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #40857:
URL: https://github.com/apache/arrow/pull/40857#issuecomment-2024356148

   Looks ok to me, but I'm not so familiar with arm. Will wait for yibo's comments


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-40806: [C++] Correctly report asimd/neon in GetRuntimeInfo [arrow]

Posted by "cyb70289 (via GitHub)" <gi...@apache.org>.
cyb70289 commented on PR #40857:
URL: https://github.com/apache/arrow/pull/40857#issuecomment-2024408269

   Thanks @amoeba !


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-40806: [C++] Correctly report asimd/neon in GetRuntimeInfo [arrow]

Posted by "cyb70289 (via GitHub)" <gi...@apache.org>.
cyb70289 merged PR #40857:
URL: https://github.com/apache/arrow/pull/40857


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-40806: [C++] Correctly report asimd/neon in GetRuntimeInfo [arrow]

Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #40857:
URL: https://github.com/apache/arrow/pull/40857#issuecomment-2026352131

   After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 6cecbab5172b2b339277dde741bfff455646eb32.
   
   There were no benchmark performance regressions. 🎉
   
   The [full Conbench report](https://github.com/apache/arrow/runs/23222958425) has more details. It also includes information about 27 possible false positives for unstable benchmarks that are known to sometimes produce them.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-40806: [C++] Correctly report asimd/neon in GetRuntimeInfo [arrow]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #40857:
URL: https://github.com/apache/arrow/pull/40857#issuecomment-2034980425

   Ok, I've opened https://github.com/apache/arrow/pull/40980 to revert.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-40806: [C++] Correctly report asimd/neon in GetRuntimeInfo [arrow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40857:
URL: https://github.com/apache/arrow/pull/40857#issuecomment-2024338347

   :warning: GitHub issue #40806 **has been automatically assigned in GitHub** to PR creator.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-40806: [C++] Correctly report asimd/neon in GetRuntimeInfo [arrow]

Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on PR #40857:
URL: https://github.com/apache/arrow/pull/40857#issuecomment-2024408534

   Thanks @cyb70289 and @mapleFU!


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org