cconvey commented on code in PR #10765: URL: https://github.com/apache/tvm/pull/10765#discussion_r841992794
########## src/runtime/rpc/rpc_module.cc: ########## @@ -21,6 +21,10 @@ * \file rpc_module.cc * \brief RPC runtime module. */ +#if defined(__hexagon__) +#define TVM_LOG_CUSTOMIZE 1 +#endif Review Comment: Nice catch about there being a corresponding CMake variable; I wasn't aware of that. And it looks like some of TVMs' build system heeds its value: https://github.com/apache/tvm/blob/6babb89cbb9fc5ab718f8b996c7ce60bf5ebbefd/cmake/modules/Logging.cmake#L20-L27 Unfortunately it looks like the Hexagon runtime is pretty consistently _not_ relying on that CMake variable: - https://github.com/apache/tvm/blob/6babb89cbb9fc5ab718f8b996c7ce60bf5ebbefd/src/runtime/hexagon/hexagon/hexagon_buffer.cc#L21-L23 - https://github.com/apache/tvm/blob/6babb89cbb9fc5ab718f8b996c7ce60bf5ebbefd/src/runtime/hexagon/hexagon/hexagon_device_api_v2.cc#L23-L26 - https://github.com/apache/tvm/blob/6babb89cbb9fc5ab718f8b996c7ce60bf5ebbefd/src/runtime/hexagon/hexagon/hexagon_common.cc#L23-L26 - https://github.com/apache/tvm/blob/6babb89cbb9fc5ab718f8b996c7ce60bf5ebbefd/src/runtime/hexagon/rpc/hexagon/rpc_server.cc#L46-L49 How about this, assuming that the final form of the PR even _uses_ that `LOG(...)` statement: - Leave the code as is, because it's consistent with the pattern used elsewhere in the Hexagon runtime codebase. - We try to tackle this issue for all 4/5 of these files in a separate PR. (IMHO it would be mistake to have this present PR also do a cleanup our the Hexagon runtime's logging scheme.) -- 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]
