mehrdadh commented on code in PR #11547:
URL: https://github.com/apache/tvm/pull/11547#discussion_r889353770


##########
python/tvm/contrib/hexagon/session.py:
##########
@@ -194,33 +196,36 @@ def get_graph_executor(
         self._set_device_type(graph_mod)
         return tvm.contrib.graph_executor.create(graph_json, graph_mod, 
self.device)
 
-    def get_aot_executor(
+    def get_graph_debug_executor(

Review Comment:
   Unfortunately github didn't present this correctly. I removed 
`get_aot_executor` function because it didn't work correctly and also it was 
not used anywhere in the codebase. Therefore it will remain broken if we keep 
it and others might try to use it.



##########
python/tvm/contrib/hexagon/pytest_plugin.py:
##########
@@ -173,19 +180,51 @@ def hexagon_launcher(
         rpc_info = {
             "rpc_tracker_host": tvm_tracker_host,
             "rpc_tracker_port": tvm_tracker_port,
-            "rpc_server_port": rpc_server_port,
+            "rpc_server_port": rpc_server_port_for_session,
             "adb_server_socket": adb_server_socket,
         }
         launcher = HexagonLauncher(serial_number=android_serial_number, 
rpc_info=rpc_info)
-        launcher.start_server()
+
         try:
+            launcher.start_server()
             yield launcher
         finally:
             launcher.stop_server()
 
 
[email protected]
-def hexagon_session(hexagon_launcher) -> Session:
[email protected]
+def hexagon_launcher(
+    hexagon_server_process,
+    rpc_server_port,
+    tvm_tracker_host,
+    tvm_tracker_port,
+    adb_server_socket,
+    android_serial_number,
+) -> HexagonLauncherRPC:
+    """Initials and returns hexagon launcher which reuses RPC info and Android 
serial number."""
+    if hexagon_server_process._serial_number != "simulator":

Review Comment:
   I think simulator is more dependent to have all the execution files in the 
same workspace and since it was already working fine, I tried not to change it. 
We could also change that in this PR or a follow up PR.



##########
python/tvm/contrib/hexagon/session.py:
##########
@@ -194,33 +196,36 @@ def get_graph_executor(
         self._set_device_type(graph_mod)
         return tvm.contrib.graph_executor.create(graph_json, graph_mod, 
self.device)
 
-    def get_aot_executor(
+    def get_graph_debug_executor(

Review Comment:
   Also I changed `get_graph_debug_executor` to work with the current approach. 
Just realized we are missing a test for that as well. I suggest to add a test 
for it since this api is useful for debugging.



##########
python/tvm/contrib/hexagon/session.py:
##########
@@ -109,7 +110,7 @@ def device(self):
 
         return self._device
 
-    def upload(self, local_path: Union[str, pathlib.Path], remote_filename: 
str):
+    def upload(self, local_path: Union[str, pathlib.Path], remote_filename: 
str) -> pathlib.Path:

Review Comment:
   I need this change to make this approach work. It is used in `load_module` 
function.



-- 
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]

Reply via email to