支持环境变量指定 MACA 架构#29
Conversation
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates get_maca_arch in mxcc.py to support specifying the MACA path via the MACA_PATH environment variable, refactors the architecture parsing logic into a helper function, and adds unit tests. The feedback highlights three key areas for improvement: catching OSError to prevent crashes when the macainfo binary is missing, avoiding global sys.modules pollution in tests by directly importing tvm, and supporting a MACA_ARCH environment variable to allow bypassing macainfo entirely in non-GPU or cross-compilation environments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| gpu_arch = match.group(1) | ||
| gpu_arch = _parse_macainfo_arch(macainfo_output) or gpu_arch | ||
| return gpu_arch.lower() | ||
| except subprocess.CalledProcessError: |
There was a problem hiding this comment.
在 get_maca_arch 中,如果 maca_path 目录存在,但其下没有 bin/macainfo 可执行文件,subprocess.check_output 将会抛出 FileNotFoundError(它是 OSError 的子类)。当前的 except subprocess.CalledProcessError: 无法捕获该异常,这会导致程序崩溃。
建议同时捕获 OSError 以提高鲁棒性。
| except subprocess.CalledProcessError: | |
| except (subprocess.CalledProcessError, OSError): |
| import importlib.util | ||
| import os | ||
| import sys | ||
| import types | ||
| import unittest | ||
| from pathlib import Path | ||
| from tempfile import TemporaryDirectory | ||
| from unittest.mock import patch | ||
|
|
||
|
|
||
| REPO_ROOT = Path(__file__).resolve().parents[3] | ||
| MXCC_PATH = REPO_ROOT / "python" / "tvm" / "contrib" / "mxcc.py" | ||
|
|
||
|
|
||
| def _register_global_func(*args, **_kwargs): | ||
| if args and callable(args[0]): | ||
| return args[0] | ||
| return lambda fn: fn | ||
|
|
||
|
|
||
| tvm_ffi = types.SimpleNamespace(register_global_func=_register_global_func) | ||
| tvm = types.ModuleType("tvm") | ||
| tvm.target = types.SimpleNamespace(Target=types.SimpleNamespace(current=lambda: None)) | ||
| tvm.maca = lambda *_args, **_kwargs: types.SimpleNamespace(exist=False) | ||
| tvm.__path__ = [] | ||
| tvm_contrib = types.ModuleType("tvm.contrib") | ||
| tvm_contrib.__path__ = [] | ||
| tvm_target = types.ModuleType("tvm.target") | ||
| tvm_target.Target = object | ||
| tvm_base = types.ModuleType("tvm.base") | ||
| tvm_base.py_str = lambda value: value.decode("utf-8") | ||
| tvm_contrib_utils = types.ModuleType("tvm.contrib.utils") | ||
| tvm_contrib_utils.tempdir = lambda: None | ||
|
|
||
| sys.modules.setdefault("tvm_ffi", tvm_ffi) | ||
| sys.modules.setdefault("tvm", tvm) | ||
| sys.modules.setdefault("tvm.contrib", tvm_contrib) | ||
| sys.modules.setdefault("tvm.target", tvm_target) | ||
| sys.modules.setdefault("tvm.base", tvm_base) | ||
| sys.modules.setdefault("tvm.contrib.utils", tvm_contrib_utils) | ||
|
|
||
| spec = importlib.util.spec_from_file_location("tvm.contrib.mxcc", MXCC_PATH) | ||
| mxcc = importlib.util.module_from_spec(spec) | ||
| mxcc.__package__ = "tvm.contrib" | ||
| sys.modules["tvm.contrib.mxcc"] = mxcc | ||
| assert spec.loader is not None | ||
| spec.loader.exec_module(mxcc) | ||
|
|
There was a problem hiding this comment.
在测试文件中,通过手动修改 sys.modules 并使用 importlib 动态加载 mxcc.py 来 mock tvm 模块。这种在模块级别全局修改 sys.modules 的做法是一个严重的测试反模式(anti-pattern)。
当使用 pytest 或其他测试运行器在同一个进程中运行整个测试套件时,这个测试文件会永久污染全局的 sys.modules。后续其他真正需要导入真实 tvm 模块的测试将会导入这个被 mock 的极简 tvm 模块,从而导致大面积的测试失败(例如 AttributeError)。
在 TVM 的测试环境中,测试是在 TVM 构建完成后运行的,因此可以直接导入 tvm。建议移除这些复杂的 mock 逻辑,直接使用标准的 import 语句。
| import importlib.util | |
| import os | |
| import sys | |
| import types | |
| import unittest | |
| from pathlib import Path | |
| from tempfile import TemporaryDirectory | |
| from unittest.mock import patch | |
| REPO_ROOT = Path(__file__).resolve().parents[3] | |
| MXCC_PATH = REPO_ROOT / "python" / "tvm" / "contrib" / "mxcc.py" | |
| def _register_global_func(*args, **_kwargs): | |
| if args and callable(args[0]): | |
| return args[0] | |
| return lambda fn: fn | |
| tvm_ffi = types.SimpleNamespace(register_global_func=_register_global_func) | |
| tvm = types.ModuleType("tvm") | |
| tvm.target = types.SimpleNamespace(Target=types.SimpleNamespace(current=lambda: None)) | |
| tvm.maca = lambda *_args, **_kwargs: types.SimpleNamespace(exist=False) | |
| tvm.__path__ = [] | |
| tvm_contrib = types.ModuleType("tvm.contrib") | |
| tvm_contrib.__path__ = [] | |
| tvm_target = types.ModuleType("tvm.target") | |
| tvm_target.Target = object | |
| tvm_base = types.ModuleType("tvm.base") | |
| tvm_base.py_str = lambda value: value.decode("utf-8") | |
| tvm_contrib_utils = types.ModuleType("tvm.contrib.utils") | |
| tvm_contrib_utils.tempdir = lambda: None | |
| sys.modules.setdefault("tvm_ffi", tvm_ffi) | |
| sys.modules.setdefault("tvm", tvm) | |
| sys.modules.setdefault("tvm.contrib", tvm_contrib) | |
| sys.modules.setdefault("tvm.target", tvm_target) | |
| sys.modules.setdefault("tvm.base", tvm_base) | |
| sys.modules.setdefault("tvm.contrib.utils", tvm_contrib_utils) | |
| spec = importlib.util.spec_from_file_location("tvm.contrib.mxcc", MXCC_PATH) | |
| mxcc = importlib.util.module_from_spec(spec) | |
| mxcc.__package__ = "tvm.contrib" | |
| sys.modules["tvm.contrib.mxcc"] = mxcc | |
| assert spec.loader is not None | |
| spec.loader.exec_module(mxcc) | |
| import os | |
| import unittest | |
| from pathlib import Path | |
| from tempfile import TemporaryDirectory | |
| from unittest.mock import patch | |
| from tvm.contrib import mxcc |
| gpu_arch = "xcore1000" | ||
| maca_path = maca_path or os.environ.get("MACA_PATH", "/opt/maca") |
There was a problem hiding this comment.
正如 PR 标题和描述所言,“支持环境变量指定 MACA 架构”。然而,目前的实现仅支持通过 MACA_PATH 环境变量指定安装路径,然后仍然依赖运行 macainfo 来获取架构。在没有物理 GPU 的编译环境或容器中,这仍然会回退到默认的 xcore1000。
建议直接支持 MACA_ARCH 环境变量(例如 MACA_ARCH=xcore2000),如果设置了该环境变量,则直接返回其值,从而避免执行 macainfo。这不仅完美契合 PR 的标题,也极大地方便了交叉编译和无 GPU 环境下的构建与测试。
gpu_arch = os.environ.get("MACA_ARCH")
if gpu_arch:
return gpu_arch.lower()
gpu_arch = "xcore1000"
maca_path = maca_path or os.environ.get("MACA_PATH", "/opt/maca")
该 PR 支持通过环境变量指定 MACA 目标架构,使同一套源码更容易在不同沐曦设备和测试镜像间切换。
这个修改面向沐曦 GPU 适配场景中比较容易影响开发、构建或验证稳定性的环节,把原来需要人工排查的问题前移到工具链、运行前检查或基准脚本中处理。实现上保持对现有默认行为的兼容,只在检测到明确配置、输入或环境异常时给出更直接的诊断,避免引入额外运行依赖,也方便维护者独立审阅该分支。
已在沐曦算力环境中完成对应分支验证,验证记录包含真实运行日志、命令输出和失败路径检查,本地归档目录为:E:/Documents/muxi/测试报告/mcTVM_new_toolchain_validation_20260608。提交分支:
mengz/use-env-maca-arch,目标仓库:MetaX-MACA/mcTVM。