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.

Sign up for GitHub

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

Jump to bottom

AVX/SSSE3/AVX512_MIC ISA cleanup #2465

Merged
merged 9 commits into from
Aug 18, 2023
Merged

AVX/SSSE3/AVX512_MIC ISA cleanup #2465

merged 9 commits into from
Aug 18, 2023

Conversation

napetrov
Copy link
Contributor

Description

Cleaning remain of dropped instruction sets

@napetrov
Copy link
Contributor Author

/intelci: run

@napetrov
Copy link
Contributor Author

/intelci: run

@napetrov
Copy link
Contributor Author

/intelci: run

@ethanglaser
Copy link
Contributor

https://github.com/search?q=repo%3Anapetrov%2Fdaal+avx512_mic&type=code
If want to remove all mentions of avx512_mic

@napetrov
Copy link
Contributor Author

https://github.com/search?q=repo%3Anapetrov%2Fdaal+avx512_mic&type=code If want to remove all mentions of avx512_mic

Not sure that i get point here. All those entries are removed and this search line runs against master branch while removal happened in feature branch.

@ethanglaser
Copy link
Contributor

https://github.com/search?q=repo%3Anapetrov%2Fdaal+avx512_mic&type=code If want to remove all mentions of avx512_mic

Not sure that i get point here. All those entries are removed and this search line runs against master branch while removal happened in feature branch.

Ah, my bad, thought I was searching on the branch but I guess GitHub search only supports searching on default/master of repo.

@ethanglaser
Copy link
Contributor

Would we want to remove/substitute avx with avx512 in bottom of INSTALL.md - https://github.com/oneapi-src/oneDAL/blob/master/INSTALL.md?

@ethanglaser
Copy link
Contributor

A few last observations - mentions of plain avx currently (your discretion is better than mine here):
https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/externals/service_blas_mkl.h#L41-L42
https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/externals/service_lapack_mkl.h#L41-L42
https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/externals/service_spblas_mkl.h#L41-L42
https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/services/compiler/generic/env_detect_features.cpp#L102-L120 - couple mentions here but its in a avx512 function so not sure if they should be cleaned
https://github.com/oneapi-src/oneDAL/blob/master/dev/bazel/config/cpudetect.cpp#L86-L120 - similar to above
https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/threading/export_mac.def#L62-L83
https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/threading/export_mac.def#L154-L185
https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/threading/export_mac.def#L205-L210
https://github.com/oneapi-src/oneDAL/blob/master/cpp/oneapi/dal/backend/primitives/intersection/intersection.hpp#L365

@napetrov
Copy link
Contributor Author

A few last observations - mentions of plain avx currently (your discretion is better than mine here): https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/externals/service_blas_mkl.h#L41-L42 https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/externals/service_lapack_mkl.h#L41-L42 https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/externals/service_spblas_mkl.h#L41-L42 https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/services/compiler/generic/env_detect_features.cpp#L102-L120 - couple mentions here but its in a avx512 function so not sure if they should be cleaned https://github.com/oneapi-src/oneDAL/blob/master/dev/bazel/config/cpudetect.cpp#L86-L120 - similar to above https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/threading/export_mac.def#L62-L83 https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/threading/export_mac.def#L154-L185 https://github.com/oneapi-src/oneDAL/blob/master/cpp/daal/src/threading/export_mac.def#L205-L210 https://github.com/oneapi-src/oneDAL/blob/master/cpp/oneapi/dal/backend/primitives/intersection/intersection.hpp#L365

Thanks!
first 3 on service functions - we will be using MKL AVX kernels on mac. This is lowest that supported there. And export_mac.def are exporting avx functions from FPK.

on CPU detects - those are again for MacOS avx identification and make no harm for us

the only questionable is last on intersect which is graph experimental functionality - changed it to AVX2 and think it would be working there as well.

@napetrov
Copy link
Contributor Author

/intelci: run

@napetrov napetrov merged commit eb6735e into oneapi-src:master Aug 18, 2023
11 checks passed
md-shafiul-alam pushed a commit to Vika-F/daal that referenced this pull request Aug 25, 2023
@napetrov @md-shafiul-alam
AVX/SSSE3/AVX512_MIC ISA cleanup (oneapi-src#2465)
ad931ad 
* cleanup avx512_mic

* cleanup ssse3

* cleanup avx

* fixing typos

* fixing / typos

* fixing make typos

* clang format

* Updating ISA in install.md

* replace AVX with AVX2 for intersect
md-shafiul-alam added a commit that referenced this pull request Sep 1, 2023
@Vika-F @Alexsandruss
@ethanglaser @napetrov @Alexandr-Solovev @aepanchi @amgrigoriev @Pahandrovich @md-shafiul-alam
Add CPU parts of the performance-related hyperparameters logic (#2461)
c60a7ec 
* Fix incorrect include in service_hash_table.h file

* Add initial implementation of the base algorithm::Hyperparameter class

* clang-format

* Add Hyperparameter* _hpar field into base Algorithm and AlgorithmContainer classes

* Implement initial version of Hyperparameters class for Covariance algorithm in DAAL

* Extend covariance::BatchImpl (and thus covariance::Batch) interface with the Hyperparameter support

* Remove outdated verionst of setArgument methods

* Add ability to pass hyperparameters from DAAL API to kernels in Covariance algorithm

* 1. Make the hyperparameter field of DAAL Algorithm class constant. 2. Implement passing hyperparameters from oneDAL to DAAL on CPU

* Fix

* Fix Java build

* Move covariance::Hyperparameter class to covariance::internal namespace

* Removal of compression functionality (#2462)

* Temporary disabling gemv test (#2463)

* AVX/SSSE3/AVX512_MIC ISA cleanup (#2465)

* cleanup avx512_mic

* cleanup ssse3

* cleanup avx

* fixing typos

* fixing / typos

* fixing make typos

* clang format

* Updating ISA in install.md

* replace AVX with AVX2 for intersect

* Remove mention of compression in docs (#2467)

* Init addition of online interfaces in Covariance (#2455)

* Add the link to oneAPI samples browser (#2470)

* infra: update binary version (#2468)

* update binary version

* more binary version updates

* Initial input for backend selection (#2396)

* Initial input for backend selection

* Update dev/make/cmplr.gnu.mkl.mk

Co-authored-by: KulikovNikita <[email protected]>

* Update dev/make/cmplr.gnu.ref.mk

Co-authored-by: KulikovNikita <[email protected]>

* Changed default backend to 'mkl'

* Buildable onedal_c

* Added clang support

* Update dev/make/cmplr.gnu.mkl.mk

Co-authored-by: KulikovNikita <[email protected]>

* Update dev/make/cmplr.gnu.ref.mk

Co-authored-by: KulikovNikita <[email protected]>

* Compiler fixes for icc, icx, vc plus clang-format

* Update dev/make/cmplr.clang.ref.mk

Co-authored-by: KulikovNikita <[email protected]>

* adding support for selecting different math/rng/service backends in case of build with bazel

* fixed build issue with kmeans serialization

* Introducing backend_config param selection to build.sh

* Create openblas.sh

* Introduce CI build for BLAS backend

* adding execute permission on openblas.sh

* Update cpp/daal/src/externals/config_ref.h

Co-authored-by: Victoriya Fedotova <[email protected]>

* Update cpp/daal/src/externals/config_ref.h

Co-authored-by: Victoriya Fedotova <[email protected]>

* Addressed part of the comments

* Clang-format

* Turned off hyperthreadig for ref config in order to use TBB default number of threads

* Addressed more comments

* Macro fixed (APPLE)

* More changes in REF RNG

* Update build.sh

* Removed 'sed' from Makefile

* Update openblas.sh

* Removed config_template from BAZEL

* Removed backend_config_header from BAZEL

* Update openblas.sh

* Replaced safe function not supported by GNU

* Fixed bugs in ref backend for OpenBLAS build

* Fixed bugs in ref backend for OpenBLAS build #2

* Reduced header file dependencies in REF backend; removed << operator for RNG (compiling issue)

* Update ci.yml

* Fixed export on symbols for OpenBLAS build

* export.def handling in bazel

* Update cpp/daal/src/externals/service_math_ref.h

Co-authored-by: Pavel Yakovlev <[email protected]>

* Added libfgortran to REF build

* removed config_template auxiliary func

* Removed libgfortran for REF backend

* Fixed BACKEND incdirs for oneAPI; addressed some comments

* Removed unnecessary includes; fixed ifdef in _DECLAR_ files

* fixed omitted ifdef in _DECLAR_ files

* Clang-format

* Clang-format fix

* Added an option to move compression to exclude list for examples

* Excluded compression examples for all configurations

* revert some macros to __intel_compiler

* replase some more macros

* add NO_FORTRAN=1 to openblas.sh script

* Update cpp/daal/src/externals/service_service_ref.h

* Fixed missed fpk symbols for oneapi examples (REF backend)

* Update cpp/daal/src/externals/service_service_ref.h

Co-authored-by: Pavel Yakovlev <[email protected]>

* Update cpp/daal/src/externals/service_service_ref.h

Co-authored-by: Pavel Yakovlev <[email protected]>

* Fixed error message in oneapi

* exclude failed examples with ref backend

* exclude examples for oneapi/cpp ifaces

* exclude mpi examples for ref backend

* Apply suggestions from code review

* AVX512_MIC cleanup

* Fixed CI pipeline

* Added more includes for ONEAPI

* Update .ci/pipeline/ci.yml

* Switch to core count for blas build

* Update .ci/env/openblas.sh

* Adding _MKL suffix for job name

* Fixing daal4py job dependency

* Attempt to fix warnings

---------

Co-authored-by: Aleksey Grigorev <[email protected]>
Co-authored-by: KulikovNikita <[email protected]>
Co-authored-by: a.maklaev <[email protected]>
Co-authored-by: Nikolay Petrov <[email protected]>
Co-authored-by: Victoriya Fedotova <[email protected]>
Co-authored-by: Pavel Yakovlev <[email protected]>
Co-authored-by: Pavel Yakovlev <[email protected]>

* resolve merge conflict while rebasing in Fix incorrect include in service_hash_table.h file

* Add ability to pass hyperparameters from DAAL API to kernels in Covariance algorithm

* minor

* update examples

* clang-format

* minor

---------

Co-authored-by: Alexander Andreev <[email protected]>
Co-authored-by: ethanglaser <[email protected]>
Co-authored-by: Nikolay Petrov <[email protected]>
Co-authored-by: Aleksandr Solovev <[email protected]>
Co-authored-by: Alexandra <[email protected]>
Co-authored-by: amgrigoriev <[email protected]>
Co-authored-by: Aleksey Grigorev <[email protected]>
Co-authored-by: KulikovNikita <[email protected]>
Co-authored-by: a.maklaev <[email protected]>
Co-authored-by: Pavel Yakovlev <[email protected]>
Co-authored-by: Pavel Yakovlev <[email protected]>
Co-authored-by: Md Shafiul Alam <[email protected]>
Co-authored-by: msa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ethanglaser ethanglaser ethanglaser approved these changes

@Alexsandruss Alexsandruss Awaiting requested review from Alexsandruss

@samir-nasibli samir-nasibli Awaiting requested review from samir-nasibli

@KulikovNikita KulikovNikita Awaiting requested review from KulikovNikita

@Vika-F Vika-F Awaiting requested review from Vika-F

@maria-Petrova maria-Petrova Awaiting requested review from maria-Petrova

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@napetrov @ethanglaser