gemini-code-assist[bot] commented on code in PR #19647:
URL: https://github.com/apache/tvm/pull/19647#discussion_r3329783769
##########
pyproject.toml:
##########
@@ -257,32 +254,37 @@ line-ending = "auto"
docstring-code-format = false
docstring-code-line-length = "dynamic"
-[tool.mypy]
-python_version = "3.9"
-show_error_codes = true
-mypy_path = ["python"]
-files = ["python/tvm"]
-namespace_packages = true
-explicit_package_bases = true
-allow_redefinition = true
-ignore_missing_imports = true
-follow_imports = "skip"
-strict_optional = false
-exclude = '''(?x)(
- ^\.venv/|
- ^build/|
- ^dist/|
- ^\.mypy_cache/|
- ^3rdparty/
-)'''
+[tool.cibuildwheel]
+# TVM cannot build 32-bit or musl targets; skip them so a bare `cibuildwheel`
+# invocation (outside the publish matrix) fails fast instead of attempting
them.
+# The workflow matrix selects the real per-platform targets via CIBW_BUILD.
+skip = "*-win32 *-manylinux_i686 *-musllinux*"
+build-verbosity = 1
+# Verify the installed wheel with a small pytest suite (post-install checks
+# such as the LLVM compile smoke test and the static-LLVM / CUDA-runtime
+# library checks). The wheel-specific expectations are passed through the
+# environment by the publishing workflow.
+test-requires = ["pytest", "numpy"]
+# Use the suite's own pytest.ini (-c) so it does not inherit the repository's
+# root pytest config/conftest, which loads tvm.testing.plugin and the full test
+# harness — too heavy for a wheel smoke check.
+test-command = "pytest -c {project}/tests/python/wheel/pytest.ini
{project}/tests/python/wheel"
-[[tool.mypy.overrides]]
-module = ["python.tvm.auto_scheduler.*"]
-ignore_errors = true
+# Per-platform build and repair steps live here (the standard cibuildwheel
+# location, mirroring tvm-ffi). The publishing workflow only layers the dynamic
+# environment (LLVM config path, CUDA runtime, dist name/version) on top.
+# set_wheel_dist.py applies the optional TestPyPI name/version override; the
+# vendored tvm-ffi is the build-time FFI dependency. auditwheel (manylinux
+# image) and delocate (cibuildwheel) are already present; only delvewheel must
+# be installed for the Windows repair.
+[tool.cibuildwheel.linux]
+before-build = 'if command -v dnf >/dev/null 2>&1; then dnf -y install
libxml2-devel; fi && python {project}/ci/scripts/package/set_wheel_dist.py
{project}/pyproject.toml && python -m pip install -v {project}/3rdparty/tvm-ffi'
+repair-wheel-command = "auditwheel repair --exclude libtvm_ffi.so --exclude
libtvm_runtime_cuda.so --exclude 'libcuda.so.*' --exclude 'libcudart.so.*'
--exclude 'libnvrtc.so.*' --exclude 'libnvrtc-builtins.so.*' -w {dest_dir}
{wheel}"
-[[tool.mypy.overrides]]
-module = ["python.tvm.runtime.*"]
-ignore_errors = true
+[tool.cibuildwheel.macos]
+before-build = 'python {project}/ci/scripts/package/set_wheel_dist.py
{project}/pyproject.toml && python -m pip install -v {project}/3rdparty/tvm-ffi'
+repair-wheel-command =
'DYLD_LIBRARY_PATH="/opt/llvm/lib${DYLD_LIBRARY_PATH:+:$DYLD_LIBRARY_PATH}"
delocate-wheel --ignore-missing-dependencies --exclude libtvm_ffi.dylib
--require-archs {delocate_archs} -w {dest_dir} -v {wheel}'
-[dependency-groups]
-lint = ["pre-commit"]
+[tool.cibuildwheel.windows]
+before-build = 'python -m pip install delvewheel && python
{project}/ci/scripts/package/set_wheel_dist.py {project}/pyproject.toml &&
python -m pip install -v {project}/3rdparty/tvm-ffi'
+repair-wheel-command = "delvewheel repair --analyze-existing --ignore-existing
--add-path C:/opt/llvm/Library/bin --exclude tvm_ffi.dll --exclude
libtvm_ffi.dll -w {dest_dir} {wheel}"
Review Comment:
On Windows, the pip `Scripts` directory might not be in the system `PATH` by
default during the `cibuildwheel` run. Running `delvewheel` directly can result
in a 'command not found' error. Running it via `python -m delvewheel` is much
more robust and guaranteed to work.
```suggestion
repair-wheel-command = "python -m delvewheel repair --analyze-existing
--ignore-existing --add-path C:/opt/llvm/Library/bin --exclude tvm_ffi.dll
--exclude libtvm_ffi.dll -w {dest_dir} {wheel}"
```
##########
CMakeLists.txt:
##########
@@ -592,6 +594,20 @@ target_include_directories(tvm_compiler PUBLIC
"$<INSTALL_INTERFACE:${CMAKE_INST
set_property(TARGET tvm_compiler APPEND PROPERTY LINK_OPTIONS
"${TVM_NO_UNDEFINED_SYMBOLS}")
set_property(TARGET tvm_compiler APPEND PROPERTY LINK_OPTIONS
"${TVM_VISIBILITY_FLAG}")
+# Work around a GNU ld (binutils) relaxation bug that miscompiles
+# R_X86_64_GOTPCRELX relocations inside very large statically-linked archives.
+# When the full LLVM static libraries are linked into libtvm_compiler.so, the
+# library is large enough that ld can relax an indirect GOT call (LLVM built
+# with -fno-plt emits these) into a direct call with an incorrect displacement.
+# The call then targets read-only data instead of the intended function and
+# crashes at runtime with a SIGSEGV inside llvm::X86Subtarget during code
+# generation. Disabling linker relaxation keeps the GOT-indirect sequences and
+# avoids the miscompilation; it is harmless when LLVM is linked dynamically.
+# See binutils bug ld/25754.
+if(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND NOT ${USE_LLVM} MATCHES
${IS_FALSE_PATTERN})
Review Comment:
It is safer to quote `${USE_LLVM}` and `${IS_FALSE_PATTERN}` in the
`MATCHES` expression. If `USE_LLVM` contains a path with spaces or special
characters, unquoted expansion can cause CMake syntax errors.
```
if(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND NOT "${USE_LLVM}" MATCHES
"${IS_FALSE_PATTERN}")
```
##########
tests/python/wheel/test_wheel.py:
##########
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Post-install checks for a built TVM wheel.
+
+Run by cibuildwheel against the installed wheel (``test-command`` in
+``[tool.cibuildwheel]``). Each check is opt-in via an environment variable so
+the same tests apply across the wheel matrix: when the variable is unset the
+check is skipped.
+"""
+
+import glob
+import os
+from pathlib import Path
+
+import pytest
+
+import tvm
+
+
+def _expect(name):
+ """Return True/False for an expectation env var, or None when unset."""
+ value = os.environ.get(name, "")
+ if value == "":
+ return None
+ return value.strip().lower() in ("1", "true", "yes", "on")
+
+
+def _libdir():
+ return Path(tvm.__file__).resolve().parent / "lib"
+
+
+def test_llvm_compile():
+ """import tvm and run a minimal LLVM compile+execute."""
+ if _expect("TVM_EXPECT_LLVM_ENABLED") is False:
+ pytest.skip("LLVM not expected in this wheel")
+ if not tvm.runtime.enabled("llvm"):
+ if _expect("TVM_EXPECT_LLVM_ENABLED"):
+ pytest.fail("llvm runtime expected but not enabled")
+ pytest.skip("llvm runtime not enabled")
+
+ import numpy as np
+ from tvm import te
+
+ n = 8
+ a = te.placeholder((n,), name="a", dtype="float32")
+ b = te.placeholder((n,), name="b", dtype="float32")
+ c = te.compute((n,), lambda i: a[i] + b[i], name="c")
+ exe = tvm.compile(te.create_prim_func([a, b, c]), target="llvm")
+
+ dev = tvm.cpu()
+ a_nd = tvm.runtime.tensor(np.arange(n, dtype="float32"), dev)
+ b_nd = tvm.runtime.tensor(np.arange(n, dtype="float32") * 2, dev)
+ c_nd = tvm.runtime.tensor(np.zeros(n, dtype="float32"), dev)
+ exe(a_nd, b_nd, c_nd)
+ np.testing.assert_allclose(c_nd.numpy(), a_nd.numpy() + b_nd.numpy(),
rtol=1e-6)
+
+
+def test_no_dynamic_llvm():
+ """When static LLVM is expected, the wheel must not ship a dynamic
libLLVM."""
+ if not _expect("TVM_EXPECT_STATIC_LLVM"):
+ pytest.skip("static LLVM not required for this wheel")
+ dynamic = glob.glob(str(_libdir() / "libLLVM*"))
+ assert not dynamic, f"wheel ships dynamic LLVM libraries: {dynamic}"
Review Comment:
On Windows, dynamic LLVM libraries are typically named `LLVM.dll` or
`LLVM-C.dll` (without the `lib` prefix). The current glob pattern `libLLVM*`
will miss these on Windows, potentially allowing a dynamic LLVM DLL to be
silently bundled. Consider using a pattern that handles Windows DLL naming
conventions.
```suggestion
pattern = "*LLVM*" if os.name == "nt" else "libLLVM*"
dynamic = glob.glob(str(_libdir() / pattern))
assert not dynamic, f"wheel ships dynamic LLVM libraries: {dynamic}"
```
##########
CMakeLists.txt:
##########
@@ -865,12 +877,26 @@ if(TVM_BUILD_PYTHON_MODULE)
# Install third-party compiled dependencies into the same lib/ dir.
if(TARGET fpA_intB_gemm)
+ tvm_set_relative_rpath(fpA_intB_gemm)
install(TARGETS fpA_intB_gemm DESTINATION "lib")
endif()
if(TARGET flash_attn)
+ tvm_set_relative_rpath(flash_attn)
install(TARGETS flash_attn DESTINATION "lib")
endif()
+ # Install prebuilt extra runtime libraries into the same lib/ dir. This is
how
+ # the separately-built CUDA runtime (libtvm_runtime_cuda.so) is bundled: the
+ # publishing flow builds it in a CUDA-enabled environment and passes its path
+ # via TVM_PACKAGE_EXTRA_LIBS, so it ships through the normal CMake install
+ # rather than a post-build wheel rewrite.
+ foreach(_extra_lib IN LISTS TVM_PACKAGE_EXTRA_LIBS)
Review Comment:
The variable `TVM_PACKAGE_EXTRA_LIBS` is used to pass extra libraries to
bundle, but it is not declared as a CMake cache variable. Declaring it as a
cache variable with a default empty value (e.g., `set(TVM_PACKAGE_EXTRA_LIBS ""
CACHE STRING "Extra libraries to package")`) is a best practice for CMake
hygiene and makes it visible in CMake GUIs/tools.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
