-
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
[Hexagon] Don't build RPC fastrpc libraries when USE_HEXAGON_RPC=OFF #9904
Conversation
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.
@kparzysz-quic this change would break the current hexagon launcher tests. USE_HEXAGON_RPC
is used to initiate two external project builds in cmake/modules/Hexagon.cmake
for hexagon and android sides. When we build the android side as an external project, this flag is not enabled. Here is a traceback of the error:
E RuntimeError: Cannot request hexagon-dev after 5 retry, last_error:Traceback (most recent call last):
E 8: TVMFuncCall
E 7: std::__1::__function::__func<tvm::runtime::$_0, std::__1::allocator<tvm::runtime::$_0>, void (tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)>::operator()(tvm::runtime::TVMArgs&&, tvm::runtime::TVMRetValue*&&)
E 6: tvm::runtime::RPCClientConnect(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, tvm::runtime::TVMArgs)
E 5: tvm::runtime::RPCConnect(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, tvm::runtime::TVMArgs)
E 4: tvm::runtime::RPCEndpoint::InitRemoteSession(tvm::runtime::TVMArgs)
E 3: tvm::runtime::RPCEndpoint::HandleUntilReturnEvent(bool, std::__1::function<void (tvm::runtime::TVMArgs)>)
E 2: tvm::runtime::RPCEndpoint::EventHandler::HandleNextEvent(bool, bool, std::__1::function<void (tvm::runtime::TVMArgs)>)
E 1: tvm::runtime::RPCEndpoint::EventHandler::HandleProcessPacket(std::__1::function<void (tvm::runtime::TVMArgs)>)
E 0: tvm::runtime::RPCEndpoint::EventHandler::HandleReturn(tvm::runtime::RPCCode, std::__1::function<void (tvm::runtime::TVMArgs)>)
E File "/home/mhessar/work/tvm/src/runtime/rpc/rpc_endpoint.cc", line 376
E RPCError: Error caught from RPC call:
E [15:04:46] /home/mhessar/work/tvm/src/runtime/rpc/rpc_endpoint.cc:523:
E ---------------------------------------------------------------
E An error occurred during the execution of TVM.
E For more information, please see: https://tvm.apache.org/docs/errors.html
E ---------------------------------------------------------------
E Check failed: (fconstructor != nullptr) is false: Cannot find session constructor tvm.contrib.hexagon.create_hexagon_session
As it is now, it breaks TVM builds that do not set that variable, because the guarded parts want to use functions that were not compiled. I don't think it's a good idea to have the TVM build automatically build any apps. It happens to work with the launcher, but it's probably a bad precedent. If anything, the reverse should be the primary scenario---apps should build TVM runtimes configured to the apps' needs. I'm going to remove the external projects from Hexagon.cmake. That should simplify the .cmake file at the expense of a bit of inconvenience. When we auto build an app, the crux of the issue is whether we want to use the currently built TVM runtime with that app. If so, then we need to make sure that the specified TVM build configuration meets the app's expectations, which is something that TVM's cmake files should not be bothered doing. Edit: added more rationale for not auto-building apps. |
This PR will not be committed as is, but I'm keeping it open until the replacement PR is created. The reason is that it doesn't disappear from the list of PRs, in case someone wants to comment on this issue. |
I think we need to simplify the |
The problem for us is that it conflicts with our downstream code. It would actually be easier to do the cleanup first, since once CI is in place, we'd have to fix more things. Are you currently running some automatic tests that use this code? |
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.