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.

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

[ Tensor ] Refactor blas/math related files into cpu backend considering arch-dep @open sesame 10/02 13:19 #2549

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

skykongkong8
Copy link
Member

@skykongkong8 skykongkong8 commented Apr 18, 2024

While during the process of implementing additional features in NEON, I found myself making unnecessary code blocks.
This is a suggestion-draft of refactorization for current blas/math related files.

DONE

  • build on x86 & unittest (w/ & w/o fp16)
  • build on android & unittest
  • for all fallbacks
  • Temporarily, cpu_backend.h itself is done by now.
  • Following step should be syncing this work with TensorV2 : [Tensor] Refactorize Tensor Class to TensorV2 @open sesame 03/26 12:26 #2500
  • Remove CBLAS params in Tensor to get rid of unnecessary dependency
  • Use valid params for functions like : sgemv, sgemm (StorageOrder, data addr, ... )
  • This is already done for tensor.cpp in this PR, while intentionally eliminating ALL TensorV2 related files
  • Replace all #include <blas_interface.h> to #include <cpu_backend.h>
  • Replace all #include <blas_neon.h> to #include <neon_impl.h> and manage fp16 functions in .cpp files
  • Include all paths for clean builds for Tizen

Final form of this PR would be like:

...
tensor
├─── cpu_backend
│   └─── cpu_backend.h (has all external functions from previous `blas_interface.h`)
│   └─── arm
│      └─── arm_compute_backend : copy of `cpu_backend.h`. selectively uses functions from `cblas_interface`, `neon_impl`, `fallback_internal`
│      └─── neon_impl_fp16 (For armv8.2+)
│      └─── neon_impl
│           ...
│   └─── x86
│      └─── x86_compute_backend : copy of `cpu_backend.h`. selectively uses functions from `cblas_interface`, `blas_avx`, `fallback_internal`
│      └─── blas_avx : AVX2_implemented (vcvt, cblas) 
│           ...
│   └─── fallback
│      └─── fallback ( !x86 & !arm ) : copy of `cpu_backend.h`. uses ALL functions from `fallback_internal`
│      └─── fallback_internal : all raw implementations without SIMD or external lib
│           ...
│   └─── cblas_interface
│      └─── cblas_interface : all cblas-related function interfaces, and params
...

and removing blas_interface.h

In short,

  1. Substitute blas_interface.h to cpu_backend.h which has virtual functions of blas_interface.h
  2. Actual implementations are implemented at arm_compute_backend, x86_compute_backend, and fallback, and they are included considering target cpu architecture. cblas.h is used for both of them for fp32 computation.
  3. There are some differences (unsupported intrinsics, or dataTypes ) along the versions, and they are managed under each arm or x86 directory.

@taos-ci
Copy link

taos-ci commented Apr 18, 2024

📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2549. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/.

@skykongkong8 skykongkong8 changed the title [ DRAFT ] [ Refactor ] Refactor blas/math related files into cpu backend [ DRAFT ] [ Refactor ] Refactor blas/math related files into cpu backend considering arch-dep Apr 18, 2024
@taos-ci
Copy link

taos-ci commented Apr 18, 2024

:octocat: cibot: @skykongkong8, A builder checker could not be completed because one of the checkers is not completed. In order to find out a reason, please go to http://ci.nnstreamer.ai/nntrainer/ci/repo-workers/pr-checker/2549-202404181618220.29115009307861-c94059c0dfb6b54f2021eb42c4aa56adbc256ee3/.

@taos-ci
Copy link

taos-ci commented Apr 23, 2024

:octocat: cibot: @skykongkong8, nntrainer/tensor/cpu_backend/x86/x86_compute_library.cpp does not include Doxygen tags such as @file @brief @author @bug. You must include the Doxygen tags in the source code. Please refer to a Doxygen manual at http://github.com/nnstreamer/TAOS-CI/blob/main/ci/doc/doxygen-documentation.md

@taos-ci
Copy link

taos-ci commented Apr 23, 2024

:octocat: cibot: @skykongkong8, The last line of a text file must have a newline character. Please append a new line at the end of the line in nntrainer/tensor/cpu_backend/fallback/cblas_fallback.h.

Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

@taos-ci
Copy link

taos-ci commented Aug 20, 2024

:octocat: cibot: @skykongkong8, nntrainer/tensor/cpu_backend/arm/neon_impl_fp16.cpp includes bug(s). Please fix incorrect coding constructs in your commit before entering a review process.

@taos-ci
Copy link

taos-ci commented Aug 20, 2024

:octocat: cibot: @skykongkong8, A builder checker could not be completed because one of the checkers is not completed. In order to find out a reason, please go to http://ci.nnstreamer.ai/nntrainer/ci/repo-workers/pr-checker/2549-202408200905540.24510407447815-807936a1f92ba590705155408e7bca6ddaac3bc7/.

Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

@skykongkong8 skykongkong8 force-pushed the refactor_cpu_backend branch 2 times, most recently from 7d6c2fc to 738bec4 Compare August 30, 2024 01:13
Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

@skykongkong8 skykongkong8 changed the title [ Tensor ] Refactor blas/math related files into cpu backend considering arch-dep [ Tensor ] Refactor blas/math related files into cpu backend considering arch-dep @open sesame 08/30 12:28 Aug 30, 2024
Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

@skykongkong8 skykongkong8 changed the title [ Tensor ] Refactor blas/math related files into cpu backend considering arch-dep @open sesame 08/30 12:28 [ Tensor ] Refactor blas/math related files into cpu backend considering arch-dep @open sesame 08/30 13:22 Aug 30, 2024
Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

@skykongkong8 skykongkong8 changed the title [ Tensor ] Refactor blas/math related files into cpu backend considering arch-dep @open sesame 08/30 13:22 [ Tensor ] Refactor blas/math related files into cpu backend considering arch-dep @open sesame 09/09 10:05 Sep 9, 2024
Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

@skykongkong8 skykongkong8 changed the title [ Tensor ] Refactor blas/math related files into cpu backend considering arch-dep @open sesame 09/09 10:05 [ Tensor ] Refactor blas/math related files into cpu backend considering arch-dep @open sesame 10/02 13:19 Oct 2, 2024
Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

1. Substitute `blas_interface.h` to `cpu_backend.h` which has virtual functions of `blas_interface.h`
2. Actual implementations are implemented at `arm_compute_backend`, `x86_compute_backend`, and `fallback`, and they are included considering target cpu architecture. `cblas.h` is used for both of them for fp32 computation.
3. There are some differences (unsupported intrinsics, or dataTypes ) along the versions, and they are managed under each `arm` or `x86` directory.

**Self evaluation:**
1. Build test:     [X]Passed [ ]Failed [ ]Skipped
2. Run test:     [X]Passed [ ]Failed [ ]Skipped

Signed-off-by: skykongkong8 <[email protected]>
Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants