You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/04/27 17:33:13 UTC
[GitHub] [tvm] adstraw opened a new pull request, #11145: [Hexagon] Add support for on-device unit testing using gtest
adstraw opened a new pull request, #11145:
URL: https://github.com/apache/tvm/pull/11145
Adds Hexagon on-device unit testing support by linking the tvm runtime with gtest. Also moves `HexagonBuffer` unit tests from their original home as part of the `cpptest` executable to their new home as a Hexagon on-device unit test.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] kparzysz-quic commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r864208053
##########
cmake/modules/Hexagon.cmake:
##########
@@ -119,14 +116,23 @@ function(add_hexagon_wrapper_paths)
link_directories("${HEXAGON_TOOLCHAIN}/lib/iss")
endfunction()
-
-# Common sources for TVM runtime with Hexagon support
-file_glob_append(RUNTIME_HEXAGON_SRCS
- "${TVMRT_SOURCE_DIR}/hexagon/*.cc"
-)
-
+if(BUILD_FOR_HEXAGON OR USE_HEXAGON_RPC)
+ # Common sources for TVM runtime with Hexagon support
+ file_glob_append(RUNTIME_HEXAGON_SRCS
+ "${TVMRT_SOURCE_DIR}/hexagon/*.cc"
+ )
+else()
+ file_glob_append(RUNTIME_HEXAGON_SRCS
+ "${TVMRT_SOURCE_DIR}/hexagon/hexagon_module.cc"
+ )
+endif()
Review Comment:
~~This part disables Hexagon runtime when building for x86. Please revert this.~~
Nevermind. It was an error on my side. Sorry for the noise.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] adstraw commented on pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
adstraw commented on PR #11145:
URL: https://github.com/apache/tvm/pull/11145#issuecomment-1115124063
@kparzysz-quic please review
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan merged pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
csullivan merged PR #11145:
URL: https://github.com/apache/tvm/pull/11145
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r863086787
##########
CMakeLists.txt:
##########
@@ -598,6 +598,15 @@ endif()
target_link_libraries(tvm PRIVATE ${TVM_LINKER_LIBS} ${TVM_RUNTIME_LINKER_LIBS})
target_link_libraries(tvm_runtime PRIVATE ${TVM_RUNTIME_LINKER_LIBS})
+if(BUILD_FOR_HEXAGON AND DEFINED USE_HEXAGON_GTEST AND EXISTS ${USE_HEXAGON_GTEST})
Review Comment:
`USE_HEXAGON_GTEST` should likely be a [tvm_option](https://github.com/apache/tvm/blob/fa19d826f1c286e2c9ccc8770ff00cd71ddf49a1/CMakeLists.txt#L45) and should appear in `cmake/config.cmake` given that it effects the build,
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] supersat commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
supersat commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r863161266
##########
src/runtime/hexagon/tests/run_all_tests.cc:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#if defined(__hexagon__)
+
+#include <gtest/gtest.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <string>
+#include <vector>
+
+#include "../src/support/utils.h"
+
+namespace tvm {
+namespace runtime {
+namespace hexagon {
+
+TVM_REGISTER_GLOBAL("hexagon.run_all_tests").set_body([](TVMArgs args, TVMRetValue* rv) {
+ // gtest args are passed into this packed func as a singular string
+ // split gtest args using <space> delimiter and build argument vector
+ std::vector<std::string> parsed_args = tvm::support::Split(args[0], ' ');
+ std::vector<char*> argv;
+
+ // add executable name
+ argv.push_back(const_cast<char*>("hexagon_run_all_tests"));
+
+ // add parsed arguments
+ for (int i = 0; i < parsed_args.size(); ++i) {
+ argv.push_back(const_cast<char*>(parsed_args[i].data()));
+ }
+
+ // end of parsed arguments
+ argv.push_back(nullptr);
+
+ // set argument count
+ int argc = argv.size() - 1;
+
+ // initialize gtest with arguments and run
+ ::testing::InitGoogleTest(&argc, argv.data());
+ *rv = RUN_ALL_TESTS();
Review Comment:
GTest stream redirection relies on dup2(), which does not exist under QuRT.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] kparzysz-quic commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r864208053
##########
cmake/modules/Hexagon.cmake:
##########
@@ -119,14 +116,23 @@ function(add_hexagon_wrapper_paths)
link_directories("${HEXAGON_TOOLCHAIN}/lib/iss")
endfunction()
-
-# Common sources for TVM runtime with Hexagon support
-file_glob_append(RUNTIME_HEXAGON_SRCS
- "${TVMRT_SOURCE_DIR}/hexagon/*.cc"
-)
-
+if(BUILD_FOR_HEXAGON OR USE_HEXAGON_RPC)
+ # Common sources for TVM runtime with Hexagon support
+ file_glob_append(RUNTIME_HEXAGON_SRCS
+ "${TVMRT_SOURCE_DIR}/hexagon/*.cc"
+ )
+else()
+ file_glob_append(RUNTIME_HEXAGON_SRCS
+ "${TVMRT_SOURCE_DIR}/hexagon/hexagon_module.cc"
+ )
+endif()
Review Comment:
This part disables Hexagon runtime when building for x86. Please revert this.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] adstraw commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
adstraw commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r862150688
##########
src/runtime/hexagon/tests/run_all_tests.cc:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#if defined(__hexagon__)
+
+#include <gtest/gtest.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <string>
+#include <vector>
+
+#include "../src/support/utils.h"
+
+namespace tvm {
+namespace runtime {
+namespace hexagon {
+
+TVM_REGISTER_GLOBAL("hexagon.run_all_tests").set_body([](TVMArgs args, TVMRetValue* rv) {
+ // gtest args are passed into this packed func as a singular string
+ // split gtest args using <space> delimiter and build argument vector
+ std::vector<std::string> parsed_args = tvm::support::Split(args[0], ' ');
+ std::vector<char*> argv;
+
+ // add executable name
+ argv.push_back(const_cast<char*>("hexagon_run_all_tests"));
+
+ // add parsed arguments
+ for (int i = 0; i < parsed_args.size(); ++i) {
+ argv.push_back(const_cast<char*>(parsed_args[i].data()));
+ }
+
+ // end of parsed arguments
+ argv.push_back(nullptr);
+
+ // set argument count
+ int argc = argv.size() - 1;
+
+ // initialize gtest with arguments and run
+ ::testing::InitGoogleTest(&argc, argv.data());
+ *rv = RUN_ALL_TESTS();
Review Comment:
My understanding is that one must set `GTEST_HAS_STREAM_REDIRECTION=0` when compiling gtest for Hexagon which [disables](https://github.com/google/googletest/blob/8ded48c37be09d8cc3665af1b414c5d53c0862e7/googletest/include/gtest/internal/gtest-port.h#L1130) the `CaptureStdout` function. I believe this would be true even if we used a "stock" gtest version as opposed to the gtest version in the Hexagon SDK.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] adstraw commented on pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
adstraw commented on PR #11145:
URL: https://github.com/apache/tvm/pull/11145#issuecomment-1111585644
Open issues before publish:
1) Gtest version. Currently using version from the Hexagon SDK. May be able to use "stock" gtest version with some tweaks.
2) May need to handle case where gtest version is not available
3) May need to add a cmake option enable gtest linkage with tvm runtime with appropriate default
4) Nice-to-have an example unit test which exercises the Hexagon DeviceAPI
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r863084463
##########
src/runtime/hexagon/tests/run_all_tests.cc:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#if defined(__hexagon__)
+
+#include <gtest/gtest.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <string>
+#include <vector>
+
+#include "../src/support/utils.h"
+
+namespace tvm {
+namespace runtime {
+namespace hexagon {
+
+TVM_REGISTER_GLOBAL("hexagon.run_all_tests").set_body([](TVMArgs args, TVMRetValue* rv) {
+ // gtest args are passed into this packed func as a singular string
+ // split gtest args using <space> delimiter and build argument vector
+ std::vector<std::string> parsed_args = tvm::support::Split(args[0], ' ');
+ std::vector<char*> argv;
+
+ // add executable name
+ argv.push_back(const_cast<char*>("hexagon_run_all_tests"));
+
+ // add parsed arguments
+ for (int i = 0; i < parsed_args.size(); ++i) {
+ argv.push_back(const_cast<char*>(parsed_args[i].data()));
+ }
+
+ // end of parsed arguments
+ argv.push_back(nullptr);
+
+ // set argument count
+ int argc = argv.size() - 1;
+
+ // initialize gtest with arguments and run
+ ::testing::InitGoogleTest(&argc, argv.data());
+ *rv = RUN_ALL_TESTS();
Review Comment:
Darn. @kparzysz-quic any idea why this is disabled for Hexagon? Is there an alternative for us to capture stdout on Hexagon?
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] adstraw commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
adstraw commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r861980348
##########
CMakeLists.txt:
##########
@@ -598,6 +598,15 @@ endif()
target_link_libraries(tvm PRIVATE ${TVM_LINKER_LIBS} ${TVM_RUNTIME_LINKER_LIBS})
target_link_libraries(tvm_runtime PRIVATE ${TVM_RUNTIME_LINKER_LIBS})
+if(BUILD_FOR_HEXAGON)
+ include(FetchContent)
+ FetchContent_Declare(googletest SOURCE_DIR "${USE_HEXAGON_SDK}/utils/googletest/gtest")
+ set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
+ FetchContent_MakeAvailable(googletest)
+ target_link_libraries(tvm_runtime PUBLIC gtest)
Review Comment:
Made USE_HEXAGON_GTEST an optional cmake param and enabling / disabling Hexagon gtest feature if that cmake param is specified with a path that exists.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh commented on pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on PR #11145:
URL: https://github.com/apache/tvm/pull/11145#issuecomment-1112349109
Is there a reason that put test files under `src/runtime/hexagon/tests/` instead of `tests/cpp/...`?
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] adstraw commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
adstraw commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r863255007
##########
CMakeLists.txt:
##########
@@ -598,6 +598,15 @@ endif()
target_link_libraries(tvm PRIVATE ${TVM_LINKER_LIBS} ${TVM_RUNTIME_LINKER_LIBS})
target_link_libraries(tvm_runtime PRIVATE ${TVM_RUNTIME_LINKER_LIBS})
+if(BUILD_FOR_HEXAGON AND DEFINED USE_HEXAGON_GTEST AND EXISTS ${USE_HEXAGON_GTEST})
Review Comment:
Done.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] adstraw commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
adstraw commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r861979606
##########
src/runtime/hexagon/tests/run_all_tests.cc:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#if defined(__hexagon__)
+
+#include <gtest/gtest.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <string>
+#include <vector>
+
+#include "../src/support/utils.h"
+
+namespace tvm {
+namespace runtime {
+namespace hexagon {
+
+TVM_REGISTER_GLOBAL("hexagon.run_all_tests").set_body([](TVMArgs args, TVMRetValue* rv) {
+ // gtest args are passed into this packed func as a singular string
+ // split gtest args using <space> delimiter and build argument vector
+ std::vector<std::string> parsed_args = tvm::support::Split(args[0], ' ');
+ std::vector<char*> argv;
+
+ // add executable name
+ argv.push_back(const_cast<char*>("hexagon_run_all_tests"));
+
+ // add parsed arguments
+ for (int i = 0; i < parsed_args.size(); ++i) {
+ argv.push_back(const_cast<char*>(parsed_args[i].data()));
+ }
+
+ // end of parsed arguments
+ argv.push_back(nullptr);
+
+ // set argument count
+ int argc = argv.size() - 1;
+
+ // initialize gtest with arguments and run
+ ::testing::InitGoogleTest(&argc, argv.data());
+ *rv = RUN_ALL_TESTS();
Review Comment:
Unfortunately, it looks like stdout redirection is one of the disabled features in the Hexagon SDK gtest version.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] adstraw commented on pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
adstraw commented on PR #11145:
URL: https://github.com/apache/tvm/pull/11145#issuecomment-1113631599
> Is there a reason that you put test files under `src/runtime/hexagon/tests/` instead of `tests/cpp/...`?
Short answer: We need a separate directory from which to include runtime tests.
Long answer ...
All files in `tests/cpp/...` are recursively [globbed](https://github.com/apache/tvm/blob/c6578834bc34a8721844197df03e0cef83440adf/CMakeLists.txt#L634) and added to the `cpptest` executable. This forces all code tested by `cpptest` to have an x86 implementation. For runtime code, it may or may not be possible to have an x86 implementation.
The `HexagonBuffer` class, for example, was able to have both a Hexagon (VTMC allocation via the SDK) and x86 (malloc) implementation and hence was testable using `cpptest` on x86. However, this added some complication to the build system which is removed in this PR along with the x86 implementation for `HexagonBuffer`. `HexagonBuffer` is now implemented and tested on Hexagon.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] kparzysz-quic commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r863089842
##########
src/runtime/hexagon/tests/run_all_tests.cc:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#if defined(__hexagon__)
+
+#include <gtest/gtest.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <string>
+#include <vector>
+
+#include "../src/support/utils.h"
+
+namespace tvm {
+namespace runtime {
+namespace hexagon {
+
+TVM_REGISTER_GLOBAL("hexagon.run_all_tests").set_body([](TVMArgs args, TVMRetValue* rv) {
+ // gtest args are passed into this packed func as a singular string
+ // split gtest args using <space> delimiter and build argument vector
+ std::vector<std::string> parsed_args = tvm::support::Split(args[0], ' ');
+ std::vector<char*> argv;
+
+ // add executable name
+ argv.push_back(const_cast<char*>("hexagon_run_all_tests"));
+
+ // add parsed arguments
+ for (int i = 0; i < parsed_args.size(); ++i) {
+ argv.push_back(const_cast<char*>(parsed_args[i].data()));
+ }
+
+ // end of parsed arguments
+ argv.push_back(nullptr);
+
+ // set argument count
+ int argc = argv.size() - 1;
+
+ // initialize gtest with arguments and run
+ ::testing::InitGoogleTest(&argc, argv.data());
+ *rv = RUN_ALL_TESTS();
Review Comment:
I don't know. What happens if we set `GTEST_HAS_STREAM_REDIRECTION=1`?
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] adstraw commented on pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
adstraw commented on PR #11145:
URL: https://github.com/apache/tvm/pull/11145#issuecomment-1115223454
> for now we could consider changing the cpptest globbing to exclude the runtime subdirectory with the assumption that runtime specific tests will need to be globbed by a tvm_runtime compatible gtest impl.
I don't mind the current `src/runtime/hexagon/tests` file location for Hexagon tests but I am also open to changing it based on feedback from @areusch @mehrdadh @csullivan. Let me know what you think.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] adstraw commented on pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
adstraw commented on PR #11145:
URL: https://github.com/apache/tvm/pull/11145#issuecomment-1115464441
Given that `cpptest` recursively globs all files under `tests/cpp` I decided to make a new folder named `tests/cpp-runtime` with subdirectory for Hexagon. I am hoping this will satisfy the concerns about test location.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r861212085
##########
CMakeLists.txt:
##########
@@ -598,6 +598,15 @@ endif()
target_link_libraries(tvm PRIVATE ${TVM_LINKER_LIBS} ${TVM_RUNTIME_LINKER_LIBS})
target_link_libraries(tvm_runtime PRIVATE ${TVM_RUNTIME_LINKER_LIBS})
+if(BUILD_FOR_HEXAGON)
+ include(FetchContent)
+ FetchContent_Declare(googletest SOURCE_DIR "${USE_HEXAGON_SDK}/utils/googletest/gtest")
+ set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
+ FetchContent_MakeAvailable(googletest)
+ target_link_libraries(tvm_runtime PUBLIC gtest)
Review Comment:
Thinking we maybe should conditionally do this if a cmake variable is set so that we are only linking in gtest to the tvm_runtime when we want to do testing in order to avoid tvm_runtime bloat.
##########
src/runtime/hexagon/tests/run_all_tests.cc:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#if defined(__hexagon__)
+
+#include <gtest/gtest.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <string>
+#include <vector>
+
+#include "../src/support/utils.h"
+
+namespace tvm {
+namespace runtime {
+namespace hexagon {
+
+TVM_REGISTER_GLOBAL("hexagon.run_all_tests").set_body([](TVMArgs args, TVMRetValue* rv) {
+ // gtest args are passed into this packed func as a singular string
+ // split gtest args using <space> delimiter and build argument vector
+ std::vector<std::string> parsed_args = tvm::support::Split(args[0], ' ');
+ std::vector<char*> argv;
+
+ // add executable name
+ argv.push_back(const_cast<char*>("hexagon_run_all_tests"));
+
+ // add parsed arguments
+ for (int i = 0; i < parsed_args.size(); ++i) {
+ argv.push_back(const_cast<char*>(parsed_args[i].data()));
+ }
+
+ // end of parsed arguments
+ argv.push_back(nullptr);
+
+ // set argument count
+ int argc = argv.size() - 1;
+
+ // initialize gtest with arguments and run
+ ::testing::InitGoogleTest(&argc, argv.data());
+ *rv = RUN_ALL_TESTS();
Review Comment:
Neat! looks like gtest supports capturing the stdout to a string that we can return in the ret value across the FFI,
```
testing::internal::CaptureStdout();
// Run tests
// ...
std::string output = testing::internal::GetCapturedStdout();
```
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r862038668
##########
src/runtime/hexagon/tests/run_all_tests.cc:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#if defined(__hexagon__)
+
+#include <gtest/gtest.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <string>
+#include <vector>
+
+#include "../src/support/utils.h"
+
+namespace tvm {
+namespace runtime {
+namespace hexagon {
+
+TVM_REGISTER_GLOBAL("hexagon.run_all_tests").set_body([](TVMArgs args, TVMRetValue* rv) {
+ // gtest args are passed into this packed func as a singular string
+ // split gtest args using <space> delimiter and build argument vector
+ std::vector<std::string> parsed_args = tvm::support::Split(args[0], ' ');
+ std::vector<char*> argv;
+
+ // add executable name
+ argv.push_back(const_cast<char*>("hexagon_run_all_tests"));
+
+ // add parsed arguments
+ for (int i = 0; i < parsed_args.size(); ++i) {
+ argv.push_back(const_cast<char*>(parsed_args[i].data()));
+ }
+
+ // end of parsed arguments
+ argv.push_back(nullptr);
+
+ // set argument count
+ int argc = argv.size() - 1;
+
+ // initialize gtest with arguments and run
+ ::testing::InitGoogleTest(&argc, argv.data());
+ *rv = RUN_ALL_TESTS();
Review Comment:
How so? in that case can we target a generic gtest and then modify to support for Hexagon as we have discussed potentially doing?
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
csullivan commented on PR #11145:
URL: https://github.com/apache/tvm/pull/11145#issuecomment-1113728878
I think @areusch may argue this is a reasonable example of why we should have test files co-located with source implementations. That said, for now we could consider changing the cpptest globbing to exclude the runtime subdirectory with the assumption that runtime specific tests will need to be globbed by a tvm_runtime compatible gtest impl.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
areusch commented on PR #11145:
URL: https://github.com/apache/tvm/pull/11145#issuecomment-1115294513
@adstraw until such a future time as we colocate all the tests with the source files, i think it'd be great to keep them underneath `tests/` so we don't vary too much impl-to-impl. i'm okay with creating further directories underneath tests/cpptest as needed for this.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] supersat commented on a diff in pull request #11145: [Hexagon] Add support for on-device unit testing using gtest
Posted by GitBox <gi...@apache.org>.
supersat commented on code in PR #11145:
URL: https://github.com/apache/tvm/pull/11145#discussion_r863192174
##########
src/runtime/hexagon/tests/run_all_tests.cc:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#if defined(__hexagon__)
+
+#include <gtest/gtest.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <string>
+#include <vector>
+
+#include "../src/support/utils.h"
+
+namespace tvm {
+namespace runtime {
+namespace hexagon {
+
+TVM_REGISTER_GLOBAL("hexagon.run_all_tests").set_body([](TVMArgs args, TVMRetValue* rv) {
+ // gtest args are passed into this packed func as a singular string
+ // split gtest args using <space> delimiter and build argument vector
+ std::vector<std::string> parsed_args = tvm::support::Split(args[0], ' ');
+ std::vector<char*> argv;
+
+ // add executable name
+ argv.push_back(const_cast<char*>("hexagon_run_all_tests"));
+
+ // add parsed arguments
+ for (int i = 0; i < parsed_args.size(); ++i) {
+ argv.push_back(const_cast<char*>(parsed_args[i].data()));
+ }
+
+ // end of parsed arguments
+ argv.push_back(nullptr);
+
+ // set argument count
+ int argc = argv.size() - 1;
+
+ // initialize gtest with arguments and run
+ ::testing::InitGoogleTest(&argc, argv.data());
+ *rv = RUN_ALL_TESTS();
Review Comment:
This may be useful: https://google.github.io/googletest/advanced.html#extending-googletest-by-handling-test-events
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org