Skip to content

规范化 MACA 架构版本#25

Open
ghangz wants to merge 1 commit into
MetaX-MACA:mainfrom
ghangz:mengz/normalize-maca-arch-version
Open

规范化 MACA 架构版本#25
ghangz wants to merge 1 commit into
MetaX-MACA:mainfrom
ghangz:mengz/normalize-maca-arch-version

Conversation

@ghangz

@ghangz ghangz commented Jun 8, 2026

Copy link
Copy Markdown

该 PR 对 MACA 架构版本字符串做规范化处理,减少大小写、前缀和格式差异对编译参数生成的影响。

这个修改面向沐曦 GPU 适配场景中比较容易影响开发、构建或验证稳定性的环节,把原来需要人工排查的问题前移到工具链、运行前检查或基准脚本中处理。实现上保持对现有默认行为的兼容,只在检测到明确配置、输入或环境异常时给出更直接的诊断,避免引入额外运行依赖,也方便维护者独立审阅该分支。

已在沐曦算力环境中完成对应分支验证,验证记录包含真实运行日志、命令输出和失败路径检查,本地归档目录为:E:/Documents/muxi/测试报告/mcTVM_new_toolchain_validation_20260608。提交分支:mengz/normalize-maca-arch-version,目标仓库:MetaX-MACA/mcTVM

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a helper function normalize_maca_arch in python/tvm/contrib/mxcc.py to normalize MACA architecture strings into compute version strings, refactoring get_target_compute_version to use it. It also adds a unit test file tests/python/contrib/test_mxcc_arch_normalize.py to verify the normalization logic. The review feedback highlights two main issues: first, normalize_maca_arch will raise a RuntimeError on valid architecture names that contain trailing alphabetic suffixes (e.g., xcore1000b), which can be resolved by stripping these suffixes using regex; second, the test file's approach of dynamically parsing and executing code via ast and exec is fragile and should be replaced with standard imports and standard pytest assertions.

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.

Comment on lines +157 to +164
arch = arch.lower()
if arch.startswith("xcore"):
arch = arch[len("xcore") :]
if "." in arch:
parse_compute_version(arch)
return arch
if not arch.isdigit() or len(arch) < 3:
raise RuntimeError("MACA architecture parsing error: " + arch)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

get_maca_arch 函数中,通过正则 r"Name:\s+(XCORE\d+[a-zA-Z]*)" 获取 GPU 架构时,允许架构名称末尾带有字母后缀(例如 XCORE1000B)。如果返回了带有字母后缀的架构(如 xcore1000b),在经过 normalize_maca_arch 处理时,由于 not arch.isdigit() 的限制,会直接抛出 RuntimeError 异常。

建议在解析前使用正则表达式剥离末尾的字母后缀,以提高对带后缀架构名称的兼容性和鲁棒性。

Suggested change
arch = arch.lower()
if arch.startswith("xcore"):
arch = arch[len("xcore") :]
if "." in arch:
parse_compute_version(arch)
return arch
if not arch.isdigit() or len(arch) < 3:
raise RuntimeError("MACA architecture parsing error: " + arch)
arch = arch.lower()
if arch.startswith("xcore"):
arch = arch[len("xcore") :]
arch = re.sub(r"[a-z]+$", "", arch)
if "." in arch:
parse_compute_version(arch)
return arch
if not arch.isdigit() or len(arch) < 3:
raise RuntimeError("MACA architecture parsing error: " + arch)

Comment on lines +1 to +43
import ast
from pathlib import Path


def _load_arch_helpers():
source_path = (
Path(__file__).resolve().parents[3] / "python" / "tvm" / "contrib" / "mxcc.py"
)
tree = ast.parse(source_path.read_text(encoding="utf-8"))
wanted = {"parse_compute_version", "normalize_maca_arch"}
nodes = [
node
for node in tree.body
if isinstance(node, ast.FunctionDef) and node.name in wanted
]
module = ast.Module(body=nodes, type_ignores=[])
ast.fix_missing_locations(module)
namespace = {}
exec(compile(module, str(source_path), "exec"), namespace)
return namespace


def test_normalize_maca_arch_accepts_xcore_and_compute_version():
helpers = _load_arch_helpers()

assert helpers["normalize_maca_arch"]("xcore1000") == "10.0"
assert helpers["normalize_maca_arch"]("xcore1030") == "10.30"
assert helpers["normalize_maca_arch"]("10.0") == "10.0"


def test_normalize_maca_arch_rejects_unknown_string():
helpers = _load_arch_helpers()
try:
helpers["normalize_maca_arch"]("xcore")
except RuntimeError as err:
assert "architecture parsing" in str(err)
else:
raise AssertionError("expected RuntimeError")


if __name__ == "__main__":
test_normalize_maca_arch_accepts_xcore_and_compute_version()
test_normalize_maca_arch_rejects_unknown_string()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

当前测试文件使用 ast.parseexec 动态加载并执行 mxcc.py 中的函数。这种做法非常脆弱且难以维护:

  1. 如果被测试函数未来引用了模块级别的全局变量、导入的模块(如 reos 等)或 TVM 的其他部分,动态执行时会因为命名空间缺失而抛出 NameError
  2. 这种非标准的测试编写方式增加了代码复杂度,降低了可读性。

由于测试是在 TVM 构建/运行环境中执行的,建议直接通过标准的 import 语句导入 normalize_maca_arch 进行测试。同时,可以增加对带字母后缀架构(如 xcore1000b)的测试用例,并使用更符合 Python 惯例的 pytest.raises 来断言异常。

Suggested change
import ast
from pathlib import Path
def _load_arch_helpers():
source_path = (
Path(__file__).resolve().parents[3] / "python" / "tvm" / "contrib" / "mxcc.py"
)
tree = ast.parse(source_path.read_text(encoding="utf-8"))
wanted = {"parse_compute_version", "normalize_maca_arch"}
nodes = [
node
for node in tree.body
if isinstance(node, ast.FunctionDef) and node.name in wanted
]
module = ast.Module(body=nodes, type_ignores=[])
ast.fix_missing_locations(module)
namespace = {}
exec(compile(module, str(source_path), "exec"), namespace)
return namespace
def test_normalize_maca_arch_accepts_xcore_and_compute_version():
helpers = _load_arch_helpers()
assert helpers["normalize_maca_arch"]("xcore1000") == "10.0"
assert helpers["normalize_maca_arch"]("xcore1030") == "10.30"
assert helpers["normalize_maca_arch"]("10.0") == "10.0"
def test_normalize_maca_arch_rejects_unknown_string():
helpers = _load_arch_helpers()
try:
helpers["normalize_maca_arch"]("xcore")
except RuntimeError as err:
assert "architecture parsing" in str(err)
else:
raise AssertionError("expected RuntimeError")
if __name__ == "__main__":
test_normalize_maca_arch_accepts_xcore_and_compute_version()
test_normalize_maca_arch_rejects_unknown_string()
import pytest
from tvm.contrib.mxcc import normalize_maca_arch
def test_normalize_maca_arch_accepts_xcore_and_compute_version():
assert normalize_maca_arch("xcore1000") == "10.0"
assert normalize_maca_arch("xcore1030") == "10.30"
assert normalize_maca_arch("10.0") == "10.0"
assert normalize_maca_arch("xcore1000b") == "10.0"
def test_normalize_maca_arch_rejects_unknown_string():
with pytest.raises(RuntimeError, match="architecture parsing"):
normalize_maca_arch("xcore")
if __name__ == "__main__":
test_normalize_maca_arch_accepts_xcore_and_compute_version()
test_normalize_maca_arch_rejects_unknown_string()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant