echuraev commented on code in PR #13670:
URL: https://github.com/apache/tvm/pull/13670#discussion_r1059279528
##########
cmake/modules/contrib/CLML.cmake:
##########
@@ -22,7 +22,21 @@ if(USE_CLML)
if(NOT USE_CLML_GRAPH_EXECUTOR)
list(APPEND COMPILER_SRCS ${CLML_RUNTIME_MODULE})
endif()
- message(STATUS "Build with CLML support...")
+ message(STATUS "Build with CLML support : " ${USE_CLML})
+ if (NOT USE_CLML STREQUAL "ON")
+ set(CLML_VERSION_HEADER "${USE_CLML}/CL/cl_qcom_ml_ops.h")
+ 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})
Review Comment:
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)?
##########
src/runtime/contrib/clml/clml_runtime.cc:
##########
@@ -153,13 +153,24 @@ class CLMLRuntime : public JSONRuntimeBase {
ICHECK(result == CL_SUCCESS) << "clQueryMLInterfaceVersionsQCOM:" <<
result;
for (cl_uint i = 0; i < numVersions; ++i) {
+#if CL_QCOM_ML_OPS_H_MAJOR_VERSION == 2
if (majorVersions[i] == 2) {
- LOG(WARNING) << "CLML Version Selected:" << majorVersions[i] << " : "
<< majorVersions[i];
h_ClmlIntf = clGetMLInterfaceV2QCOM(0);
- ICHECK(h_ClmlIntf != NULL) << "clGetMLInterfaceV2QCOM:" << result;
+ LOG(WARNING) << "CLML Target version:" << majorVersions[i];
break;
}
+#endif
+#if CL_QCOM_ML_OPS_H_MAJOR_VERSION == 3
+ if (majorVersions[i] == 3) {
+ h_ClmlIntf = clGetMLInterfaceV3QCOM(0);
+ LOG(WARNING) << "CLML Target version:" << majorVersions[i];
+ break;
+ }
+#endif
}
+ ICHECK(h_ClmlIntf != NULL)
+ << "clGetMLInterfaceVxQCOM:" << result
+ << " Perhaps there is mispatch between CLML SDK version to target
supported version";
Review Comment:
Let's print target supported version
##########
tests/scripts/task_build_adreno_bins.sh:
##########
@@ -29,7 +29,7 @@ cd ${output_directory}
cp ../cmake/config.cmake .
echo set\(USE_MICRO OFF\) >> config.cmake
-echo set\(USE_CLML ON\) >> config.cmake
+echo set\(USE_CLML "${ADRENO_OPENCL}"\) >> config.cmake
Review Comment:
Is it not necessary to do this check (`[[ -z "${ADRENO_OPENCL}" ]] &&
CLML_PATH='ON' || CLML_PATH="${ADRENO_OPENCL}"`) here?
##########
tests/python/contrib/test_clml/infrastructure.py:
##########
@@ -181,16 +199,34 @@ def extract_clml_modules(module):
def verify_codegen(
- module,
+ mod,
known_good_codegen,
+ device,
+ params,
num_clml_modules=1,
tvm_ops=0,
- target="llvm -mtriple=aarch64-linux-gnu",
):
"""Check clml codegen against a known good output."""
- module = build_module(module, target, tvm_ops=tvm_ops,
clml_partitions=num_clml_modules)
- clml_modules = extract_clml_modules(module)
+ if isinstance(mod, tvm.relay.expr.Call):
+ mod = tvm.IRModule.from_expr(mod)
+ with tvm.transform.PassContext(opt_level=3,
disabled_pass=["AlterOpLayout"]):
+ mod = clml.partition_for_clml(mod, params)
+ tvm_op_count = get_cpu_op_count(mod)
+ assert tvm_op_count == tvm_ops, "Got {} TVM operators, expected
{}".format(
Review Comment:
What if the user didn't pass value of `tvm_ops`? Probably `tvm_ops` should
be required parameter?
##########
tests/python/contrib/test_clml/test_ops.py:
##########
@@ -54,11 +62,8 @@ def _get_conv_model(
shape = (shape[0], shape[1], shape[2] + padding[0] * 2, shape[3] +
padding[1] * 2)
is_depthwise = shape[1] == channels == groups
- weight_format = "OIHW" if is_depthwise else "OIHW"
- if weight_format == "IOHW":
- weight_shape = (shape[1] // groups, channels, kernel_h, kernel_w)
- else:
- weight_shape = (channels, shape[1] // groups, kernel_h, kernel_w)
+ weight_format = "OIHW"
Review Comment:
Why did you remove check on the dephtwise convolution?
##########
tests/python/contrib/test_clml/test_ops.py:
##########
@@ -86,31 +91,122 @@ def _get_conv_model(
if has_activation:
out = relay.nn.relu(out)
- print("Out:", out)
-
return out, params
+def _get_conv_expected_codegen(
+ shape,
+ kernel_h,
+ kernel_w,
+ padding,
+ strides,
+ dilation,
+ groups,
+ dtype,
+ channels,
+ has_bias=False,
+ has_activation=False,
+):
+ if len(padding) == 2:
+ padding = (padding[0], padding[1], padding[0], padding[1])
+ output_height = ((shape[2] - kernel_h + padding[0] + padding[2]) /
strides[0]) + 1
+ output_width = ((shape[3] - kernel_w + padding[1] + padding[3]) /
strides[1]) + 1
+ output_shape = (1, channels, int(output_height), int(output_width))
+ out_dtype = dtype
+ is_depthwise = shape[1] == channels == groups
+
+ weight_format = "IOHW" if is_depthwise else "OIHW"
+ if weight_format == "OIHW":
+ weight_shape = (channels, shape[1] // groups, kernel_h, kernel_w)
+ else:
+ weight_shape = (shape[1] // groups, channels, kernel_h, kernel_w)
+ # weight_shape = (channels, shape[1] // groups, kernel_h, kernel_w)
Review Comment:
Redundant comment
##########
tests/python/contrib/test_clml/infrastructure.py:
##########
@@ -155,9 +173,9 @@ def build_and_run(
for _ in range(no_runs):
gen_module.run()
out.append([gen_module.get_output(i) for i in range(outputs)])
- time_f = gen_module.module.time_evaluator("run", device.device.cl(0),
number=1)
- cost = time_f().mean
- print("%g secs/iteration\n" % cost)
+ # time_f = gen_module.module.time_evaluator("run", device.device.cl(0),
number=1)
+ # cost = time_f().mean
+ # print("%g secs/iteration\n" % cost)
Review Comment:
Why did you remove it?
##########
src/relay/backend/contrib/clml/codegen.cc:
##########
@@ -332,6 +332,10 @@ class CLMLJSONSerializer : public
backend::contrib::JSONSerializer {
bias = dense;
dense = dense->args[0].as<CallNode>();
}
+ if (backend::IsOp(dense, "nn.bias_add")) {
Review Comment:
Maybe unify `add` and `nn.bias_add`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]