-
Notifications
You must be signed in to change notification settings - Fork 74
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
[GPU/OpenCL] OpenCL pipeline for layer execution on GPU @open sesame 03/07 10:42 #2465
Conversation
📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2465. 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/. |
cibot: @s-debadri, 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/2465-202402061833290.23417901992798-9fac4504a9442c86c07d1fc1e9dadfb201f8bdf6/. |
nntrainer/opencl/meson.build
Outdated
|
||
foreach h : opencl_headers | ||
nntrainer_headers += meson.current_source_dir() / h | ||
endforeach |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add new line at the end of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved in next commit
nntrainer/meson.build
Outdated
@@ -42,7 +42,8 @@ nntrainer_elements = [ | |||
'optimizers', | |||
'tensor', | |||
'utils', | |||
'graph' | |||
'graph', | |||
'opencl' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be optional. how about adding option('enable-opencl', type: 'boolean', value: false)
in meson_options.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added related changes in next commit. Updated description.
cibot: @s-debadri, nntrainer/tensor/cl_operations/cl_sgemv.hpp 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 |
cibot: @s-debadri, nntrainer/opencl/opencl_buffer.cpp includes bug(s). Please fix incorrect coding constructs in your commit before entering a review process. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@s-debadri, 💯 All CI checkers are successfully verified. Thanks.
endforeach | ||
|
||
foreach h : opencl_headers | ||
nntrainer_headers += meson.current_source_dir() / h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better to use .h
for header files.
nntrainer/opencl/opencl_buffer.cpp
Outdated
|
||
#include <nntrainer_log.h> | ||
|
||
namespace nntrainer::internal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about nntrainer::opencl
namesapce rather than internal to make it clearer?
nntrainer/opencl/opencl_buffer.hpp
Outdated
#include "opencl_context_manager.hpp" | ||
#include "third_party/cl.h" | ||
|
||
namespace nntrainer::internal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about adding more comments following the oxygen to provide a better understanding?
|
||
#include <vector> | ||
|
||
#include "opencl_loader.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please check order.
cl_program prgm = program.GetProgram(); | ||
kernel_ = clCreateKernel(prgm, function_name.c_str(), &error_code); | ||
if (!kernel_ || error_code != CL_SUCCESS) { | ||
kernel_ = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a doc, clCreateKernel returns null with error code whenever it fails.
cb7ec72
to
295fb51
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@s-debadri, 💯 All CI checkers are successfully verified. Thanks.
134cb47
to
61dedf3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@s-debadri, 💯 All CI checkers are successfully verified. Thanks.
d24c2e5
to
c4de26a
Compare
cibot: @s-debadri, nntrainer/opencl/opencl_buffer.cpp includes bug(s). Please fix incorrect coding constructs in your commit before entering a review process. |
c4de26a
to
7c8a025
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@s-debadri, 💯 All CI checkers are successfully verified. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi you got 3 formatting error
clang-format reports: 3 file(s) not formatted
nntrainer/opencl/opencl_buffer.cpp
nntrainer/opencl/third_party/cl.h
nntrainer/opencl/third_party/cl_platform.h
on here, now our TAOS CI and gitaction CI has different version
so please fomatting based on gitaction CI ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@s-debadri, 💯 All CI checkers are successfully verified. Thanks.
7c8a025
to
108a81f
Compare
cibot: @s-debadri, nntrainer/opencl/opencl_buffer.cpp includes bug(s). Please fix incorrect coding constructs in your commit before entering a review process. |
108a81f
to
bc32461
Compare
cibot: @s-debadri, nntrainer/opencl/third_party/cl_platform.h 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 |
cibot: @s-debadri, nntrainer/opencl/opencl_buffer.cpp includes bug(s). Please fix incorrect coding constructs in your commit before entering a review process. |
@s-debadri |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@s-debadri, 💯 All CI checkers are successfully verified. Thanks.
Fixed CI issues for the following: Third party files: clang Non third party files: clang, doxygen Signed-off-by: Debadri Samaddar <[email protected]>
c3e642e
to
7403a02
Compare
cibot: @s-debadri, nntrainer/opencl/third_party/cl_platform.h 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 |
cibot: @s-debadri, nntrainer/opencl/opencl_buffer.cpp includes bug(s). Please fix incorrect coding constructs in your commit before entering a review process. |
Handled conditions by reducting ifdef checks Signed-off-by: Debadri Samaddar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@s-debadri, 💯 All CI checkers are successfully verified. Thanks.
I found this commit emits error when using ndk-build in the current main ld: error: undefined symbol: nntrainer::ClContext::Global()
>>> referenced by layers_dependent_common_tests.cpp:38 (../unittest/layers/layers_dependent_common_tests.cpp:38)
>>> /home/sungsik/nntrainer/test/obj/local/arm64-v8a/objs/unittest_layers/__/unittest/layers/layers_dependent_common_tests.o:(LayerSemantics_createFromClContext_pn_Test::TestBody())
ld: error: undefined symbol: int const nntrainer::ClContext::registerFactory<nntrainer::Layer>(std::__ndk1::function<std::__ndk1::unique_ptr<nntrainer::Layer, std::__ndk1::default_delete<nntrainer::Layer> > (std::__ndk1::vector<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, std::__ndk1::allocator<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > > > const&)>, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, int)
>>> referenced by layers_dependent_common_tests.cpp:40 (../unittest/layers/layers_dependent_common_tests.cpp:40)
>>> /home/sungsik/nntrainer/test/obj/local/arm64-v8a/objs/unittest_layers/__/unittest/layers/layers_dependent_common_tests.o:(LayerSemantics_createFromClContext_pn_Test::TestBody())
ld: error: undefined symbol: nntrainer::opencl::ContextManager::ReleaseContext()
>>> referenced by layer_context.h:829 (/home/sungsik/nntrainer/nntrainer/layers/layer_context.h:829)
>>> /home/sungsik/nntrainer/test/obj/local/arm64-v8a/objs/unittest_layers/__/unittest/layers/layers_golden_tests.o:(nntrainer::RunLayerContext::~RunLayerContext())
clang++: error: linker command failed with exit code 1 (use -v to see invocation) |
@skykongkong8
|
@s-debadri |
This PR includes the initial version of OpenCL pipeline for layers to run on GPU. Following wrappers have been added which uses OpenCL APIs internally:
opencl_loader
: Loading required OpenCL functions.opencl_context_manager
: Getting device details and managing context creation globally.opencl_command_queue_manager
: Creation and deletion of global command queue, reading and writing buffers, dispatching kernels.opencl_kernel
: Managing kernels and setting arguments.opencl_program
: Building and managing OpenCL program.opencl_buffer
: Creating, reading and writing data on OpenCL buffer.opencl_op_interface
has been created to handle the workflow of kernel execution.Tensor level changes:
cl_interface
to handle tensor operations which will be executed on GPU.sum
andsum_by_batch
function signatures to use boolean flag for choosing compute engine (CPU/GPU).enable-opencl
flag incorporated inmeson.options
Update (23 Feb 2024):
ml::train::LayerComputeEngine
enum added to handle compute engine information. OnlyFullyConnected
layer API signature modified as of now.createLayer
API modified to propagate compute engine info viafactory
tolayer_node
.cl_context
created to handle global configuration of OpenCL environment which will create command queue and context for OpenCL and register layers to be run on GPU.layer_context
along with utility to create kernel.cl_context
global instance creation with GPU command queue.Signed-off-by: Debadri Samaddar [email protected]