[PATCH] Static libg++ linking fix #854
Conversation
- Added static linking flags for GCC and libstdc++ in CMakeLists.txt to enhance library linking. - Removed the cmake version requirement from pyproject.toml to allow for broader compatibility. - Updated the tox command in the Docker distribution script to include Python 3.8 for testing environments.
- Changed Python version requirement in README.md from 3.9+ to 3.8+. - Updated installation and testing scripts to use Python 3.8 instead of 3.9, ensuring compatibility with the new minimum version. - Adjusted tox commands in local and PyPI distribution scripts to include Python 3.8 in the testing environments.
…ect.toml - Added CMake version requirement (>=3.26) to pyproject.toml for build compatibility. - Created a Python 3.8 environment in the Dockerfile and added a symlink for easier access to the Python 3.8 executable.
- Removed static linking flags from CMakeLists.txt to simplify build configuration. - Updated Dockerfile to use Ubuntu 20.04 and streamlined the installation of dependencies, removing gcc-9 and g++-9. - Adjusted symlink creation for Python environments to use the `-sf` option for safer linking.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughVersion string bumped to 0.1.6.post1; Docker build base and packaging flow switched to manylinux/manylinux-style images with GPU support and updated system packages/Conda env creation; global CMake flags for -static-libgcc/-static-libstdc++ removed in favor of per-target static stdlib linkage; tox/pyproject adjustments for manylinux. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer / CI
participant Docker as Docker Build
participant Container as Build Container
participant Conda as Conda envs (py38..py312)
participant Tox as Tox / setup.py
participant Wheel as Wheel / auditwheel
Dev->>Docker: docker build (pypi.manylinux.Dockerfile / manylinux builder)
Docker->>Container: start image (cuda12.1 / manylinux)
Container->>Conda: create py38..py312 envs + symlinks
Container->>Tox: run tox envs (py*-pypi) (isolated_build=False)
Tox->>Wheel: python setup.py bdist_wheel --plat-name=manylinux2014_x86_64
Wheel->>Container: auditwheel repair -> manylinux2014_x86_64
Note over Container,Tox: Docker run includes --gpus all
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
👋 Hi! Thank you for contributing to the TileLang project. Please remember to run We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀 |
Summary of ChangesHello @LeiWang1999, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily focuses on updating the build environment for PyPI packages by upgrading the base Ubuntu image in the Dockerfile from 18.04 to 20.04. This update streamlines compiler management and enhances the robustness of symbolic link creation for Python executables, contributing to a more stable and modern build process. The version has also been bumped to Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the PyPI wheel building environment to Ubuntu 20.04 to address a linking issue. However, this change introduces a critical problem: building wheels on a modern OS like Ubuntu 20.04 while tagging them as manylinux1_x86_64 will produce packages that are not widely compatible, violating the manylinux standard. My primary feedback is to use an official manylinux Docker image for building the wheels. I've also included a minor suggestion to improve the robustness of one of the Dockerfile's RUN commands.
| @@ -1,19 +1,15 @@ | |||
| FROM nvidia/cuda:12.1.0-devel-ubuntu18.04 | |||
| FROM nvidia/cuda:12.1.0-devel-ubuntu20.04 | |||
There was a problem hiding this comment.
Building wheels and tagging them as manylinux1_x86_64 (as seen in maint/scripts/pypi_distribution.sh) on an Ubuntu 20.04 base image is incorrect and will lead to compatibility issues. The manylinux standards exist to ensure that wheels are compatible with a wide range of Linux distributions by building against an older, stable set of core libraries (like glibc).
Ubuntu 20.04 has a much newer glibc than what is permitted by manylinux1 (or even newer standards like manylinux2014). This will result in wheels that fail to load on many systems with a GLIBC_... not found error.
To correctly build manylinux wheels, you must use the official manylinux Docker images provided by PyPA, such as quay.io/pypa/manylinux2014_x86_64. Please switch the base image to a suitable manylinux image to ensure your wheels are portable.
| RUN conda tos accept --override-channels --channel https://repo.anaconda.com/pkgs/main; \ | ||
| conda tos accept --override-channels --channel https://repo.anaconda.com/pkgs/r |
There was a problem hiding this comment.
The change from && to ; means that the second conda tos accept command will execute even if the first one fails. In RUN instructions, it's generally safer to use && to chain commands. This ensures that the build process fails immediately if a step is unsuccessful, preventing downstream errors that can be harder to debug. If accepting the first ToS fails, it's better for the build to stop. I recommend reverting this to use &&.
RUN conda tos accept --override-channels --channel https://repo.anaconda.com/pkgs/main && \
conda tos accept --override-channels --channel https://repo.anaconda.com/pkgs/r
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
maint/scripts/pypi.Dockerfile (3)
6-13: Tighten APT layer: drop unused PPA, add --no-install-recommends, and clean lists.You add the toolchain PPA but don’t install from it; keep image lean and reproducible. Also collapse the ninja install, disable recommends, and purge apt lists.
Apply this diff:
-RUN set -eux; \ - apt-get update; \ - apt-get install -y software-properties-common; \ - add-apt-repository ppa:ubuntu-toolchain-r/test -y; \ - apt-get update; \ - apt-get install -y wget curl libtinfo-dev zlib1g-dev libssl-dev build-essential \ - libedit-dev libxml2-dev git; \ +RUN set -eux; \ + apt-get update; \ + apt-get install -y --no-install-recommends \ + software-properties-common wget curl \ + libtinfo-dev zlib1g-dev libssl-dev build-essential \ + libedit-dev libxml2-dev git ninja-build; \ curl -O https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh; \ bash Miniconda3-latest-Linux-x86_64.sh -b -p /miniconda3; \ - rm Miniconda3-latest-Linux-x86_64.sh; - -RUN apt-get update && apt-get install -y ninja-build + rm Miniconda3-latest-Linux-x86_64.sh; \ + rm -rf /var/lib/apt/lists/*Also applies to: 17-17
13-15: Supply-chain hardening: verify Miniconda installer checksum.Add a sha256 check to avoid TOCTOU/supply-chain risk during curl+bash.
- curl -O https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh; \ + curl -sSLO https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh; \ + curl -sSL https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh.sha256 | sha256sum -c -; \
21-23: Conda base ops LGTM; pinning channels/versions would improve reproducibility.The steps are clear. Consider pinning Miniconda and critical tools (cmake/patchelf) to known-good versions for repeatable builds.
Also applies to: 37-37
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
VERSION(1 hunks)maint/scripts/pypi.Dockerfile(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: format-check
- GitHub Check: bot-task
🔇 Additional comments (2)
maint/scripts/pypi.Dockerfile (1)
22-23: Conda TOS acceptance — confirmed.Anaconda's conda-anaconda-tos plugin and official docs show the exact command
conda tos accept --override-channels --channel <CHANNEL>; no change required.VERSION (1)
1-1: VERSION (0.1.6.post1) is PEP 440‑compliant — confirm tags/docs align
- VERSION contains "0.1.6.post1" (PEP 440 OK).
- setup.py reads/versioned from VERSION via find_version/get_tilelang_version (packaging uses VERSION as single source).
- pyproject.toml has no explicit version; no version assignments found; no git tag matching 0.1.6* discovered.
- Action: create a matching git tag for the release (e.g., 0.1.6 or v0.1.6) and update any docs or exported in‑code version constants if you intend them to be visible.
| @@ -1,19 +1,15 @@ | |||
| FROM nvidia/cuda:12.1.0-devel-ubuntu18.04 | |||
| FROM nvidia/cuda:12.1.0-devel-ubuntu20.04 | |||
There was a problem hiding this comment.
🧩 Analysis chain
Base image change may break manylinux wheel compatibility.
Building wheels on Ubuntu 20.04 (glibc 2.31) can yield non‑manylinux wheels, limiting installability across distros. If these images produce PyPI wheels, consider building in manylinux2014/2_28 and statically linking libstdc++ there; otherwise, clearly gate this path for internal use only.
🌐 Web query:
What is the current manylinux policy for PyPI wheels and recommended base images (manylinux2014, manylinux_2_28), and why building on Ubuntu can cause non‑compliant wheels?
💡 Result:
Short answer
- manylinux policy: wheels must be built so their extension modules only use symbol versions and link only to system libs allowed by the chosen manylinux tag (e.g. manylinux2014 or manylinux_2_28). manylinux2014 is based on CentOS‑7/UBI7-style toolchain constraints; manylinux_2_28 (and newer manylinux_2_xx) are based on distributions whose libc and toolchain provide at least GLIBC 2.28/etc. Use the official quay.io/pypa/manylinux_* images (manylinux2014, manylinux_2_28, manylinux_2_34, …) as build environments to produce compliant wheels. [1][2][3][4]
Why those images / tags matter
- Each manylinux tag encodes an ABI/whitelist (allowed system libraries and symbol versions). Building inside the associated manylinux container ensures your compiled .so files only use symbols present on older target systems and therefore can be uploaded/installed with that manylinux tag. The manylinux project README lists available tags and the runtime/pip compatibility expectations. [2][3]
Why building on Ubuntu (or any newer distro) can produce non‑compliant wheels
- Ubuntu (and other newer distros) ship newer glibc/libstdc++ and newer symbol versions. Compiling on those systems can produce binaries that reference “too‑recent” versioned symbols (e.g. GLIBC_2.28+, GLIBCXX symbols) that are not available on the older systems manylinux tags guarantee compatibility for. auditwheel can detect and “repair” some external library dependencies, but it cannot fix binaries that were compiled against a newer libc/symbol version — in that case you must build on an older toolchain (i.e. the manylinux container) or target a newer manylinux tag. See auditwheel’s docs and issue threads explaining “too‑recent versioned symbols” errors. [4][5][6]
Practical recommendations
- Build wheels inside the official manylinux Docker images matching the tag you want to publish (quay.io/pypa/manylinux2014_x86_64, quay.io/pypa/manylinux_2_28_x86_64, …). Use auditwheel to check/repair and choose a manylinux tag appropriate for your minimum supported target systems. Note manypackagers have migrated to manylinux_2_28 and newer tags (which raise the minimum glibc/toolchain). [2][3][5]
Sources
- PEP 599 (manylinux2014 policy). [1]
- pypa/manylinux README (available manylinux images, compatibility table). [2]
- cibuildwheel options (defaults and recommendation to use manylinux images; future default manylinux_2_28). [3]
- auditwheel documentation (show/repair and explanation why compiling on newer toolchains breaks compatibility). [4]
- auditwheel / manylinux issues and project announcements (examples of “too‑recent versioned symbols” and migration to manylinux_2_28). [5][6]
Base image change may break manylinux wheel compatibility.
File: maint/scripts/pypi.Dockerfile (line 1: FROM nvidia/cuda:12.1.0-devel-ubuntu20.04) — Ubuntu 20.04 (glibc 2.31) can produce binaries that reference newer glibc/glibcxx symbol versions, making wheels non‑manylinux and unrecoverable by auditwheel. If this image is used to produce PyPI wheels, rebuild inside the official manylinux containers matching your target tag (quay.io/pypa/manylinux_2_28_x86_64 or quay.io/pypa/manylinux2014_x86_64), run auditwheel repair, or gate these builds for internal use only.
🤖 Prompt for AI Agents
In maint/scripts/pypi.Dockerfile around line 1, the base image uses
nvidia/cuda:12.1.0-devel-ubuntu20.04 which can produce wheels that violate
manylinux symbol versioning; update the build to run inside the official
manylinux containers (e.g. quay.io/pypa/manylinux_2_28_x86_64 or
quay.io/pypa/manylinux2014_x86_64) or otherwise gate the CUDA/Ubuntu-based image
for internal-only use, and ensure you run auditwheel repair on produced wheels
before publishing to PyPI; concretely either change the Dockerfile to use the
appropriate quay.io/pypa/manylinux... image and install CUDA/toolchain there, or
move packaging steps into a manylinux container and invoke auditwheel repair,
documenting the internal-only exception if you cannot switch to manylinux.
| RUN set -eux; \ | ||
| apt-get update; \ | ||
| # Install gcc-9 and g++-9 | ||
| apt-get install -y software-properties-common; \ | ||
| add-apt-repository ppa:ubuntu-toolchain-r/test -y; \ | ||
| apt-get update; \ | ||
| apt-get install -y wget curl libtinfo-dev zlib1g-dev libssl-dev build-essential \ | ||
| libedit-dev libxml2-dev git gcc-9 g++-9; \ | ||
| # Switch default gcc/g++ to new version | ||
| update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-9 100; \ | ||
| update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-9 100; \ | ||
| update-alternatives --install /usr/bin/cc cc /usr/bin/gcc 100; \ | ||
| update-alternatives --install /usr/bin/c++ c++ /usr/bin/g++ 100; \ | ||
| gcc --version; g++ --version; \ | ||
| libedit-dev libxml2-dev git; \ | ||
| curl -O https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh; \ |
There was a problem hiding this comment.
🧩 Analysis chain
Static libstdc++ availability: verify libstdc++.a exists.
For “static libg++ linking fix,” ensure the static archive is present for the chosen GCC. build-essential usually pulls it via libstdc++--dev, but verify to avoid link failures.
🏁 Script executed:
#!/bin/bash
# Inspect where static libstdc++ would live on Ubuntu images
printf 'Expect at least one libstdc++.a path under /usr/lib/gcc/*/*/\n'
grep -R --line-number --files-with-matches -E '^!<arch>' /usr/lib/gcc 2>/dev/null | sed -n '1,10p' || trueLength of output: 60
Install static libstdc++ (libstdc++.a) in the image
maint/scripts/pypi.Dockerfile (lines 6–13): append libstdc++--dev (or libstdc++-dev / g++-) to the apt-get install line — verification returned no libstdc++.a under /usr/lib/gcc; the static libstdc++ is missing and will cause static-link failures.
🤖 Prompt for AI Agents
In maint/scripts/pypi.Dockerfile around lines 6 to 13, the image is missing the
static libstdc++ (libstdc++.a) which causes static-link failures; update the
apt-get install invocation to include the development package for libstdc++ (for
example append libstdc++-dev or libstdc++-<version>-dev, or add the matching
g++-<version> package) so the static library is installed under /usr/lib/gcc;
ensure the chosen package matches the Ubuntu toolchain repo you added.
| ln -sf /miniconda3/envs/py38/bin/python3.8 /usr/bin/python3.8; \ | ||
| ln -sf /miniconda3/envs/py39/bin/python3.9 /usr/bin/python3.9; \ | ||
| ln -sf /miniconda3/envs/py310/bin/python3.10 /usr/bin/python3.10; \ | ||
| ln -sf /miniconda3/envs/py311/bin/python3.11 /usr/bin/python3.11; \ | ||
| ln -sf /miniconda3/envs/py312/bin/python3.12 /usr/bin/python3.12; \ |
There was a problem hiding this comment.
Don’t write into /usr/bin with Conda pythons.
Symlinking Conda interpreters under /usr/bin can confuse system tools and apt scripts. Prefer /usr/local/bin or no global links—activate envs or use conda run.
Apply this diff:
- ln -sf /miniconda3/envs/py38/bin/python3.8 /usr/bin/python3.8; \
- ln -sf /miniconda3/envs/py39/bin/python3.9 /usr/bin/python3.9; \
- ln -sf /miniconda3/envs/py310/bin/python3.10 /usr/bin/python3.10; \
- ln -sf /miniconda3/envs/py311/bin/python3.11 /usr/bin/python3.11; \
- ln -sf /miniconda3/envs/py312/bin/python3.12 /usr/bin/python3.12; \
+ ln -sf /miniconda3/envs/py38/bin/python3.8 /usr/local/bin/python3.8; \
+ ln -sf /miniconda3/envs/py39/bin/python3.9 /usr/local/bin/python3.9; \
+ ln -sf /miniconda3/envs/py310/bin/python3.10 /usr/local/bin/python3.10; \
+ ln -sf /miniconda3/envs/py311/bin/python3.11 /usr/local/bin/python3.11; \
+ ln -sf /miniconda3/envs/py312/bin/python3.12 /usr/local/bin/python3.12; \Or drop these links entirely and rely on conda run -n pyXY python.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ln -sf /miniconda3/envs/py38/bin/python3.8 /usr/bin/python3.8; \ | |
| ln -sf /miniconda3/envs/py39/bin/python3.9 /usr/bin/python3.9; \ | |
| ln -sf /miniconda3/envs/py310/bin/python3.10 /usr/bin/python3.10; \ | |
| ln -sf /miniconda3/envs/py311/bin/python3.11 /usr/bin/python3.11; \ | |
| ln -sf /miniconda3/envs/py312/bin/python3.12 /usr/bin/python3.12; \ | |
| ln -sf /miniconda3/envs/py38/bin/python3.8 /usr/local/bin/python3.8; \ | |
| ln -sf /miniconda3/envs/py39/bin/python3.9 /usr/local/bin/python3.9; \ | |
| ln -sf /miniconda3/envs/py310/bin/python3.10 /usr/local/bin/python3.10; \ | |
| ln -sf /miniconda3/envs/py311/bin/python3.11 /usr/local/bin/python3.11; \ | |
| ln -sf /miniconda3/envs/py312/bin/python3.12 /usr/local/bin/python3.12; \ |
🤖 Prompt for AI Agents
In maint/scripts/pypi.Dockerfile around lines 32-36 the Dockerfile creates
symlinks from Conda-managed interpreters into /usr/bin which can interfere with
system tools; remove those /usr/bin symlink lines and either (a) create symlinks
under /usr/local/bin instead, or (b) drop the links entirely and update any
downstream commands to use conda run -n pyXY python (or activate the env) so the
image does not mutate /usr/bin; make the change consistent for all listed Python
versions and update any documentation or commands in the Dockerfile that assumed
/usr/bin paths.
- Eliminated static linking flags for GCC and libstdc++ to simplify build configuration and avoid potential conflicts with Python extensions.
- Changed base image from `tilelang-builder:18.04` to `tilelang-builder:manylinux` in both local and PyPI distribution scripts. - Updated Dockerfile references to use `pypi.manylinux.Dockerfile`. - Added `--gpus all` flag to the Docker run command to enable GPU support during execution.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
tilelang/transform/add_bufstore_wrapper.py (3)
71-89: Indices typing + accumulation bug; fix IntImm handling.
- node.indices elements are PrimExpr, not int → type hint is misleading.
- Multiple accesses to the same buffer overwrite prior indices; you only validate the last one.
- Comparing IntImm to 0 via index != 0 yields a PrimExpr, which raises on truth-testing.
Apply this diff to accumulate all index lists and fix typing and IntImm handling:
-def collect_buffer_indices(statement) -> Dict[Buffer, List[int]]: +def collect_buffer_indices(statement) -> Dict[Buffer, List[list]]: @@ - buffer_to_indices = {} + buffer_to_indices: Dict[Buffer, List[list]] = {} @@ - if isinstance(node, (BufferLoad, BufferStore)): - buffer_to_indices[node.buffer] = node.indices + if isinstance(node, (BufferLoad, BufferStore)): + idx_list = list(node.indices) + buffer_to_indices.setdefault(node.buffer, []).append(idx_list) @@ - return buffer_to_indices + return buffer_to_indicesAnd add precise typing outside this hunk:
from tvm.tir import PrimExpr from typing import List, Dict # Dict[Buffer, List[List[PrimExpr]]]
134-145: Validate all fragment indices; avoid PrimExpr truthiness.Validate every index list for fragment buffers and use IntImm.value:
- buffer_indices = collect_buffer_indices(statement) - for buffer, indices in buffer_indices.items(): - if buffer.scope() == "local.fragment": - for index in indices: - if isinstance(index, IntImm) and index != 0: - raise ValueError( - f"Fragment buffer access with non-zero index [{index}] is not supported. " - "Only fragment[0] access is allowed.") + buffer_indices = collect_buffer_indices(statement) + for buffer, idx_lists in buffer_indices.items(): + # Only allow fragment[0] access + if (buffer.scope() if not callable(getattr(buffer, "scope", None)) else buffer.scope()) == "local.fragment": + for indices in idx_lists: + for index in indices: + if not isinstance(index, IntImm) or index.value != 0: + raise ValueError( + f"Fragment buffer access with non-zero index [{index}] is not supported. " + "Only fragment[0] access is allowed.")
108-111: Thread binding var extraction may be brittle.AttrStmt.node can be Var or IterVar depending on pipeline stage. Accessing node.var will fail for Var. Consider:
node = statement.node thread_binding_vars.add(getattr(node, "var", node))tilelang/utils/sparse.py (1)
76-92: Ensure block_k is supplied when compress() routes to compress_sm90compress_sm90 requires block_k; compress() currently forwards **kwargs with no default — callers that don't pass block_k will raise TypeError on SM90.
Locations:
- tilelang/utils/sparse.py:76-92 (compress() routing to compress_sm90).
- examples/gemm_sp/example_gemm_sp.py:136-138 — calls compress(...) without block_k (will fail on SM90).
- testing/python/tilelibrary/test_tilelang_tilelibrary_gemm_sp.py:231 — calls compress(..., block_k=block_K) (OK).
Options:
- Provide an explicit default in compress(), e.g.:
- if compute_version >= (9, 0): - return compress_sm90(A, transposed=transposed, **kwargs) + if compute_version >= (9, 0): + bk = kwargs.pop("block_k", 128) + return compress_sm90(A, block_k=bk, transposed=transposed, **kwargs)
- Or raise a clear error requiring callers to pass block_k when targeting SM90.
🧹 Nitpick comments (9)
tilelang/transform/add_bufstore_wrapper.py (1)
44-69: Typing migration OK; confirm Buffer.scope() API across TVM versions.Some TVM versions expose Buffer.scope as a property, others as a method. If it's a property, calls like buffer.scope() will fail at runtime. Consider normalizing with getattr or checking both.
Example (outside this hunk):
scope = buffer.scope() if callable(getattr(buffer, "scope", None)) else getattr(buffer, "scope", "")tilelang/utils/sparse.py (1)
46-56: Clamp warning shows clamped value; preserve original for message.The warning currently prints 128 after clamping. Capture the original first:
- if block_k > 128: - block_k = 128 - warnings.warn( - f"block_k {block_k} is too large, set to 128 for sm90 compression.", stacklevel=2) + if block_k > 128: + orig_block_k = block_k + block_k = 128 + warnings.warn( + f"block_k {orig_block_k} is too large; clamped to 128 for sm90 compression.", stacklevel=2)tox.ini (1)
6-6: Considerskip_install = trueto speed up packaging envs.You’re not importing the package in these tox envs; skipping install reduces work.
-skip_install = false +skip_install = trueAlso applies to: 17-17
maint/scripts/docker_local_distribute.sh (1)
9-9: Make--gpus allconditional for portability.Running on non‑GPU hosts will fail. Detect GPU support first.
-docker run --rm --gpus all -v $(pwd):/tilelang ${IMAGE} /bin/bash -c "$install_pip && $tox_command" +GPU_FLAG=$([ -x "$(command -v nvidia-smi)" ] && echo "--gpus all") +docker run --rm ${GPU_FLAG} -v "$(pwd)":/tilelang ${IMAGE} /bin/bash -c "$install_pip && $tox_command"maint/scripts/docker_pypi_distribute.sh (1)
9-9: Gate--gpus alllike in local script.Same portability concern; apply the conditional GPU flag pattern.
maint/scripts/pypi.manylinux.Dockerfile (4)
3-5: Drop Debian-specificDEBIAN_FRONTEND; not used on yum-based images.-ENV DEBIAN_FRONTEND=noninteractive \ - TZ=Etc/UTC +ENV TZ=Etc/UTC
6-12: Be cautious withyum -y updateandepel-release.Updates/EPEL can pull newer libs and risk policy drift. Prefer minimal installs; clean caches.
-RUN set -eux; \ - yum -y update && yum install -y \ +RUN set -eux; \ + yum install -y \ zlib-devel openssl-devel \ libedit-devel libxml2-devel \ - bzip2 bzip2-devel xz xz-devel \ - epel-release + bzip2 bzip2-devel xz xz-devel && \ + yum clean all && rm -rf /var/cache/yum/*
13-24: Conda env setup looks fine; add cleanup to slim image.conda create -n py312 python=3.12 -y && \ ln -sf /opt/conda/envs/py312/bin/python3.12 /usr/bin/python3.12 && \ - conda install -y cmake patchelf + conda install -y cmake patchelf && \ + conda clean -afy
13-24: Ensure PATH includes conda before build.If not already in base, set PATH.
+ENV PATH=/opt/conda/bin:${PATH}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
maint/scripts/docker_local_distribute.sh(1 hunks)maint/scripts/docker_pypi_distribute.sh(1 hunks)maint/scripts/pypi.manylinux.Dockerfile(1 hunks)pyproject.toml(0 hunks)tilelang/transform/add_bufstore_wrapper.py(3 hunks)tilelang/utils/sparse.py(4 hunks)tox.ini(1 hunks)
💤 Files with no reviewable changes (1)
- pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: format-check
🔇 Additional comments (13)
tilelang/transform/add_bufstore_wrapper.py (1)
4-4: PEP 585 compatibility fix LGTM.Using typing.Tuple/List/Dict improves 3.8+ compatibility without runtime impact.
tilelang/utils/sparse.py (1)
4-4: Typing import update LGTM.Tuple usage aligns with public signatures changed below.
tox.ini (7)
10-12: CMake python hints look good.Providing both
PYTHON_EXECUTABLEandPython3_EXECUTABLEhelps CMake find Python reliably. No action.
20-21: Env hints for PyPI builds look good.
3-3: Confirm intent forisolated_build = False.Note:
python -m buildstill isolates by default unless--no-isolationis used. Ensure this matches your toolchain expectations.
25-26: Rename toaudit_manylinux2014is clear.Naming aligns with target policy.
23-23: Don't pre‑tag wheels via setup.py; use PEP 517 build and let auditwheel retag.Using --plat-name risks conflicting tags; build a wheel with PEP 517 and then repair with auditwheel.
File: tox.ini — line 23
- python setup.py bdist_wheel --plat-name=manylinux2014_x86_64 + python -m build --wheel -o {toxinidir}/distVerification: I couldn't verify — no dist/*.whl found when checking. After building, confirm pre‑repair wheel filenames with:
ls -1 dist/*.whl | xargs -n1 basename
33-33: Fix auditwheel invocation: correct -L usage, use -w, and restrict glob to wheels
- Don't use -L=/lib — auditwheel exposes -L/--lib-sdir (default ".libs") and expects a relative subdirectory; an absolute path can escape the wheel tree. (snyk.io)
- Write repaired wheels to a wheelhouse with -w instead of relying on in-place behavior; CI tooling (cibuildwheel) uses -w {dest_dir}. (cibuildwheel.pypa.io)
- Use dist/.whl (not dist/) to avoid matching sdists. (github.com)
- --exclude takes sonames (e.g. libcuda.so.1); wildcard support was added in auditwheel ≥6.2 — do not pass '/usr/local/cuda*' as a path. (gitmemories.com)
Replace the command in tox.ini (line 33) with, for example:
bash -c 'mkdir -p wheelhouse && auditwheel repair --plat manylinux2014_x86_64 -w wheelhouse --exclude libcuda.so.1 dist/*.whl'Likely an incorrect or invalid review comment.
23-23: Add post-build auditwheel check to ensure libstdc++/libgcc are statically linked.Run auditwheel on produced wheels and fail if auditwheel reports libstdc++.so.6 or libgcc_s.so.1. Add this check to tox.ini (after the bdist_wheel step at lines 23 and 33) or to the CI release job:
#!/bin/sh set -euo pipefail found=0 for whl in wheelhouse/*.whl dist/*.whl; do [ -e "$whl" ] || continue found=1 echo "== $whl" if ! command -v auditwheel >/dev/null 2>&1; then echo "auditwheel not found; install it (e.g. pip install auditwheel)"; exit 1 fi auditwheel show "$whl" if auditwheel show "$whl" | grep -Eqi 'lib(stdc\+\+|gcc_s)\.so'; then echo "Found dynamic libstdc++/libgcc in $whl"; exit 1 fi done [ "$found" -eq 0 ] && echo "No wheels found in wheelhouse/ or dist/" && exit 0 echo "All checked wheels: no dynamic libstdc++/libgcc found."maint/scripts/docker_local_distribute.sh (1)
2-3: Switch to manylinux builder is appropriate.maint/scripts/docker_pypi_distribute.sh (2)
2-4: manylinux builder + Dockerfile ref update looks good.
7-7: tox envs are correct; consider aligning with non‑PyPI build.If you adopt
python -m buildin tox.ini, these env names stay valid.maint/scripts/pypi.manylinux.Dockerfile (1)
1-1: Confirm manylinux compatibility of base image.Ensure
pytorch/manylinux-builder:cuda12.1provides manylinux2014 (glibc 2.17) before emittingmanylinux2014wheels. Run locally and paste outputs:docker run --rm pytorch/manylinux-builder:cuda12.1 cat /etc/os-release docker run --rm pytorch/manylinux-builder:cuda12.1 ldd --version docker run --rm pytorch/manylinux-builder:cuda12.1 python3 -c 'import platform; print(platform.libc_ver())' || true docker run --rm pytorch/manylinux-builder:cuda12.1 auditwheel --version || trueIf glibc != 2.17, change the wheel target accordingly.
* bump version to 0.1.6 * phaseout py38 * py39 * Update submodule 'tvm' to latest commit adc0e48 * [Build] Update CMake and Python environment settings - Added static linking flags for GCC and libstdc++ in CMakeLists.txt to enhance library linking. - Removed the cmake version requirement from pyproject.toml to allow for broader compatibility. - Updated the tox command in the Docker distribution script to include Python 3.8 for testing environments. * [Build] Update Python version requirements in scripts and documentation - Changed Python version requirement in README.md from 3.9+ to 3.8+. - Updated installation and testing scripts to use Python 3.8 instead of 3.9, ensuring compatibility with the new minimum version. - Adjusted tox commands in local and PyPI distribution scripts to include Python 3.8 in the testing environments. * [Build] Update Python and CMake requirements in Dockerfile and pyproject.toml - Added CMake version requirement (>=3.26) to pyproject.toml for build compatibility. - Created a Python 3.8 environment in the Dockerfile and added a symlink for easier access to the Python 3.8 executable. * [Build] Update CMake and Dockerfile for improved compatibility - Removed static linking flags from CMakeLists.txt to simplify build configuration. - Updated Dockerfile to use Ubuntu 20.04 and streamlined the installation of dependencies, removing gcc-9 and g++-9. - Adjusted symlink creation for Python environments to use the `-sf` option for safer linking. * [Build] Bump version to 0.1.6.post1 for post-release updates * [Build] Remove static linking flags from CMakeLists.txt - Eliminated static linking flags for GCC and libstdc++ to simplify build configuration and avoid potential conflicts with Python extensions. * [Build] Update Docker distribution scripts for manylinux compatibility - Changed base image from `tilelang-builder:18.04` to `tilelang-builder:manylinux` in both local and PyPI distribution scripts. - Updated Dockerfile references to use `pypi.manylinux.Dockerfile`. - Added `--gpus all` flag to the Docker run command to enable GPU support during execution. * lint fix * add cmake
1 participant
as title.
Summary by CodeRabbit
Chores
Notes