-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[CLML] Version compatibility and various test cases #13670
Conversation
Codegen verification test cases for all the ops (convolution, concat, pad, pool ..etc.) that are supported by clml BYOC path. Fix depthwise conv2d issue with layout
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
bc82a06
to
1b002e0
Compare
1b002e0
to
32ce3bf
Compare
e84da6b
to
c82af19
Compare
c82af19
to
a85e777
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.
Thanks for the PR. Several comments
if(EXISTS ${CLML_VERSION_HEADER}) | ||
file(READ ${CLML_VERSION_HEADER} ver) | ||
string(REGEX MATCH "CL_QCOM_ML_OPS_H_MAJOR_VERSION ([0-9]*)" _ ${ver}) | ||
set(CLML_VERSION_MAJOR ${CMAKE_MATCH_1}) |
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.
Is it possible situation that we build TVM with a new version of CLML but our target device doesn't support it? Maybe it is better to restrict version of CLML as it was done for OpenCL (we use OpenCL 1.2)?
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.
For codegen, OpenCL1.2 restriction is good as we don't have any requirement of using features from higher versions.
CLML has newer versions with new operators support and is essential to support to get perf improvements on new gerations.
} | ||
ICHECK(h_ClmlIntf != NULL) | ||
<< "clGetMLInterfaceVxQCOM:" << result | ||
<< " Perhaps there is mispatch between CLML SDK version to target supported version"; |
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.
Let's print target supported version
# time_f = gen_module.module.time_evaluator("run", device.device.cl(0), number=1) | ||
# cost = time_f().mean | ||
# print("%g secs/iteration\n" % cost) |
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.
Why did you remove it?
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 affects CI time and also to get ideal numbers we need to set number
to a higher value too.
For now, I am leaving it for developers to uncomment and use.
bd58646
to
a85e777
Compare
258f240
to
9469b05
Compare
* [CLML][TEST] Codegen test cases for ops Codegen verification test cases for all the ops (convolution, concat, pad, pool ..etc.) that are supported by clml BYOC path. Fix depthwise conv2d issue with layout * * lint errors * * version compatilibility changes. * * review comments * * Make the adreno container compatible w/ and w/o CLML SDK availability Co-authored-by: Siva Rama Krishna Reddy B <sivb@qti.qualcomm.com>
CLML version compatibility changes to support future versions.
Codegen verification test cases for all the ops (convolution, concat, pad, pool ..etc.) that are supported by clml BYOC path.
Fix depthwise conv2d issue with layout
Adreno container is now free from CLML SDK dependency. We can use it for texture only builds now.