[Refactor] Refactor some build related configurations #827
Conversation
- Updated `pyproject.toml` and `requirements-build.txt` to specify Cython version as `Cython>=3.0.0`. - Removed unnecessary dependencies from the build system. - Enhanced `pypi.Dockerfile` to install gcc-9 and g++-9, and added ninja-build for improved build performance. - Updated conda environment creation to include Python 3.9 to 3.12, while removing the Python 3.8 environment.
|
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. WalkthroughReworks the Docker build to install GCC-9/G++-9, accept conda TOS, install ninja, and drop Python 3.8 env; pins Cython>=3.0.0 in packaging; exposes tl_mma GEMM symbols under namespace Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CMake as CMake
participant Config as Build options
participant Target as tilelang_static
participant Linker as Linker
Config->>CMake: set TILE_LANG_STATIC_STDCPP ON/OFF
CMake->>Target: configure target
alt TILE_LANG_STATIC_STDCPP && GNU
CMake->>Target: apply target_link_options(-static-libstdc++ -static-libgcc)
Note right of Target #D0E8D0: static libstdc++ applied per-target
else
Note right of Target #F0E0D0: no per-target static flags
end
Target->>Linker: link (with/without static flags)
sequenceDiagram
autonumber
participant Docker as Docker build
participant Apt as apt/PPAs
participant Conda as Miniconda
participant Env as Conda envs
Docker->>Apt: add PPA, install gcc-9/g++-9, update-alternatives (cc/c++)
Apt->>Docker: confirm compiler versions
Docker->>Conda: download & install Miniconda
Docker->>Conda: accept conda TOS (main & R)
Docker->>Docker: install ninja-build
Conda->>Env: create py39,py310,py311,py312 (py38 removed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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! 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @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 focuses on refactoring various build-related configurations and dependencies to streamline the project's build process and update its environment. Key changes include specifying a newer GCC/G++ compiler in the Dockerfile, adjusting Python environment support, and refining Python package requirements. Additionally, minor code refactoring has been applied to CUDA GEMM templates for improved readability and access.
Highlights
- Dockerfile Refinements: The
pypi.Dockerfilenow explicitly installs and setsgcc-9andg++-9as default compilers, includesninja-build, and accepts Anaconda Terms of Service. Python 3.8 environment creation has been removed. - Dependency Updates: Build and runtime dependencies have been adjusted.
pyproject.tomlhas streamlined itsrequireslist by removingcmake,torch,tox,auditwheel, andninja, whilerequirements-build.txtnow explicitly listsninjaand bothpyproject.tomlandrequirements.txtupdateCythonto version3.0.0or newer. - CUDA Template Refactoring: CUDA GEMM template headers (
gemm_sm120.h,gemm_sm80.h,gemm_sm89.h) have been refactored to introduceusingdeclarations within thetlnamespace, simplifying access totl_mmaGEMM functions.
Using Gemini Code Assist
The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request refactors various build-related configurations. The changes include updating the pypi.Dockerfile to use gcc-9, removing Python 3.8 support, and adjusting build dependencies in pyproject.toml and requirements-*.txt files. The changes to pyproject.toml correctly separate build-system requirements from build-time dependencies.
My review focuses on improving the Dockerfile and resolving inconsistencies. I've suggested optimizations to reduce Docker image size and pointed out a redundant dependency installation. Most importantly, I've identified a critical issue where a build script will fail due to the removal of the Python 3.8 environment. The C++ header changes are straightforward and look good.
| # Create environments | ||
| RUN set -eux; \ | ||
| conda create -n py38 python=3.8 -y; \ | ||
| conda create -n py39 python=3.9 -y; \ |
There was a problem hiding this comment.
With the removal of the Python 3.8 environment, the script maint/scripts/docker_pypi_distribute.sh will fail as it explicitly uses python3.8. Please update that script to use one of the available Python versions (e.g., 3.9) to ensure the build process does not break.
| RUN set -eux; \ | ||
| apt-get update; \ | ||
| apt-get install -y wget curl libtinfo-dev zlib1g-dev libssl-dev build-essential libedit-dev libxml2-dev git; \ | ||
| # 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; \ | ||
| 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 | ||
| rm Miniconda3-latest-Linux-x86_64.sh; |
There was a problem hiding this comment.
To reduce the Docker image size, it's a good practice to clean up the apt cache after installing packages. You can add apt-get clean && rm -rf /var/lib/apt/lists/* at the end of this RUN command.
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; \
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; \
apt-get clean; \
rm -rf /var/lib/apt/lists/*;
| rm Miniconda3-latest-Linux-x86_64.sh | ||
| rm Miniconda3-latest-Linux-x86_64.sh; | ||
|
|
||
| RUN apt-get update && apt-get install -y ninja-build |
There was a problem hiding this comment.
This ninja-build installation is redundant because ninja is also being added to requirements-build.txt, which will install it via pip. To avoid duplication and rely on a single package management method for this tool, it's recommended to remove this line and let pip handle the installation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
requirements-build.txt (1)
12-12: Avoid duplicate Ninja sources (pip vs apt).You install system ninja via apt in maint/scripts/pypi.Dockerfile (ninja-build) and also list pip “ninja” here. Pick one to prevent version drift. Given the Dockerfile, drop the pip dependency.
Apply this diff:
- ninha +ninjaOops, wrong line—apply this instead:
- ninha +ninjaCorrection: apply the actual removal:
- ninhaFinal, correct diff:
- ninjaAlso update the header comment (“Should be mirrored in pyproject.toml”) or remove it—these lists now intentionally diverge.
pyproject.toml (1)
3-9: Minimize build-system requirements (remove non-Python tools).“patchelf” is a system binary; listing it here makes pip try to resolve a PyPI package that isn’t needed to import the backend. Keep [build-system.requires] to what the backend needs to run.
Apply this diff:
requires = [ "build", "packaging", "setuptools>=61", "wheel", - "patchelf", "Cython>=3.0.0", ]src/tl_templates/cuda/gemm_sm80.h (1)
5-9: LGTM: clean re-export of tl_mma symbols.Public aliases under tl are helpful and non-invasive. Consider centralizing these aliases in a shared header to avoid repetition across sm80/sm89/sm120, but not blocking.
src/tl_templates/cuda/gemm_sm120.h (1)
5-9: LGTM: consistent API surface across arch headers.Same optional suggestion as sm80: a single aliasing header could reduce duplication.
maint/scripts/pypi.Dockerfile (2)
21-21: Single source of Ninja.Since you apt-install ninja-build here, remove pip “ninja” from requirements-build.txt (see that comment) to avoid conflicts. Alternatively, drop this apt step and keep pip “ninja”—but don’t keep both.
Apply this diff if you choose pip ninja:
-RUN apt-get update && apt-get install -y ninja-build +# Ninja provided by pip (requirements-build.txt); system install not needed
25-27: Pin or guard 'conda tos accept' — requires conda ≥25.5 (conda-anaconda-tos plugin)File: maint/scripts/pypi.Dockerfile
Lines: 25-27# ✅ Accept Anaconda Terms of Service for both required channels 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'conda tos accept' is provided by the conda-anaconda-tos plugin and appeared in conda 25.5.x (Anaconda shipped conda v25.5.1); pin the base image to conda ≥25.5 or add a version check/install the plugin before running these commands.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
maint/scripts/pypi.Dockerfile(1 hunks)pyproject.toml(1 hunks)requirements-build.txt(2 hunks)requirements.txt(1 hunks)src/tl_templates/cuda/gemm_sm120.h(1 hunks)src/tl_templates/cuda/gemm_sm80.h(1 hunks)src/tl_templates/cuda/gemm_sm89.h(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/tl_templates/cuda/gemm_sm89.h (3)
src/tl_templates/cuda/gemm_sm120.h (1)
tl(5-9)src/tl_templates/cuda/gemm_sm80.h (1)
tl(5-9)src/tl_templates/cuda/gemm_mma.h (4)
tl(447-482)gemm_rs(463-469)gemm_sr(474-480)gemm_ss(452-458)
src/tl_templates/cuda/gemm_sm80.h (3)
src/tl_templates/cuda/gemm_sm120.h (1)
tl(5-9)src/tl_templates/cuda/gemm_sm89.h (1)
tl(9-13)src/tl_templates/cuda/gemm_mma.h (4)
tl(447-482)gemm_rs(463-469)gemm_sr(474-480)gemm_ss(452-458)
src/tl_templates/cuda/gemm_sm120.h (3)
src/tl_templates/cuda/gemm_sm80.h (1)
tl(5-9)src/tl_templates/cuda/gemm_sm89.h (1)
tl(9-13)src/tl_templates/cuda/gemm_mma.h (4)
tl(447-482)gemm_rs(463-469)gemm_sr(474-480)gemm_ss(452-458)
⏰ 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). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: format-check
- GitHub Check: bot-task
🔇 Additional comments (5)
requirements-build.txt (1)
2-2: Good: pin Cython to v3.This aligns build tooling with Cython 3 semantics.
pyproject.toml (1)
8-8: Good: add Cython>=3.0.0 to build-system.This is sufficient for PEP 517 builds that cythonize extensions.
maint/scripts/pypi.Dockerfile (1)
41-41: LGTM.WORKDIR is fine.
src/tl_templates/cuda/gemm_sm89.h (1)
9-13: LGTM: re-exports match tl_mma and keep names uniform.Re-exports appear only in src/tl_templates/cuda/gemm_sm80.h, src/tl_templates/cuda/gemm_sm89.h, and src/tl_templates/cuda/gemm_sm120.h; implementations live in src/tl_templates/cuda/gemm_mma.h (namespace tl::tl_mma). No other tl::gemm_* declarations detected — no ABI risk.
requirements.txt (1)
2-2: Remove Cython from runtime requirements; keep it as a build-time dependencypyproject.toml already lists Cython under [build-system] and the repo contains a .pyx (tilelang/jit/adapter/cython/cython_wrapper.pyx). I found no Python-level
import Cython, but the cython adapter can invoke the cython compiler at runtime (tilelang/jit/adapter/cython/adapter.py). If you ship pre-built wheels with compiled extensions, remove Cython from runtime requirements; if you expect runtime/source compilation (execution_backend="cython"), keep it.Locations: requirements.txt (remove), pyproject.toml (keep), setup.py (build logic), tilelang/jit/adapter/cython/adapter.py, tilelang/jit/adapter/cython/cython_wrapper.pyx, MANIFEST.in.
Proposed change:
- Cython>=3.0.0
| # 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; \ |
There was a problem hiding this comment.
❓ Verification inconclusive
Base OS and toolchain approach is brittle; prefer CUDA 12.1 on Ubuntu 20.04 and drop PPA.
Ubuntu 18.04 is EOL; PPAs for it are unreliable. CUDA 12.1 images exist for ubuntu20.04, which already provide GCC 9.x so you can skip the PPA and alternatives dance.
Apply this diff to simplify and improve reproducibility:
-FROM nvidia/cuda:12.1.0-devel-ubuntu18.04
+FROM nvidia/cuda:12.1.0-devel-ubuntu20.04
@@
-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; \
+RUN set -eux; \
+ export DEBIAN_FRONTEND=noninteractive; \
+ apt-get update; \
+ apt-get install -y --no-install-recommends wget curl ca-certificates \
+ libtinfo-dev zlib1g-dev libssl-dev build-essential libedit-dev libxml2-dev git \
+ gcc g++; \
+ gcc --version; g++ --version; \
@@
- rm Miniconda3-latest-Linux-x86_64.sh;
+ echo "Verifying Miniconda checksum"; \
+ curl -sSLO https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh.sha256; \
+ sha256sum -c Miniconda3-latest-Linux-x86_64.sh.sha256; \
+ rm Miniconda3-latest-Linux-x86_64.sh Miniconda3-latest-Linux-x86_64.sh.sha256; \
+ rm -rf /var/lib/apt/lists/*Check the CUDA base tag and avoid surprises:
🏁 Script executed:
#!/bin/bash
# Verify the Docker tag exists
curl -s 'https://registry.hub.docker.com/v2/repositories/nvidia/cuda/tags/?page_size=1000' \
| jq -r '.results[].name' | rg -n '^12\.1\.0-devel-ubuntu20\.04$'Length of output: 0
Switch base to nvidia/cuda:12.1.0-devel-ubuntu20.04 and drop the gcc PPA/update-alternatives. Ubuntu 18.04 moved to ESM on May 31, 2023; NVIDIA provides CUDA 12.1 devel/runtime images for Ubuntu 20.04 and Focal ships GCC 9 by default — so remove the PPA and simplify installs as in the diff below. (lists.ubuntu.com)
Apply this diff to simplify and improve reproducibility:
-FROM nvidia/cuda:12.1.0-devel-ubuntu18.04
+FROM nvidia/cuda:12.1.0-devel-ubuntu20.04
@@
-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; \
+RUN set -eux; \
+ export DEBIAN_FRONTEND=noninteractive; \
+ apt-get update; \
+ apt-get install -y --no-install-recommends wget curl ca-certificates \
+ libtinfo-dev zlib1g-dev libssl-dev build-essential libedit-dev libxml2-dev git \
+ gcc g++; \
+ gcc --version; g++ --version; \
@@
- rm Miniconda3-latest-Linux-x86_64.sh;
+ echo "Verifying Miniconda checksum"; \
+ curl -sSLO https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh.sha256; \
+ sha256sum -c Miniconda3-latest-Linux-x86_64.sh.sha256; \
+ rm Miniconda3-latest-Linux-x86_64.sh Miniconda3-latest-Linux-x86_64.sh.sha256; \
+ rm -rf /var/lib/apt/lists/*📝 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.
| # 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; \ | |
| # Install build dependencies (no PPA) | |
| export DEBIAN_FRONTEND=noninteractive; \ | |
| apt-get update; \ | |
| apt-get install -y --no-install-recommends wget curl ca-certificates \ | |
| libtinfo-dev zlib1g-dev libssl-dev build-essential \ | |
| libedit-dev libxml2-dev git gcc g++; \ | |
| gcc --version; g++ --version; \ |
🤖 Prompt for AI Agents
In maint/scripts/pypi.Dockerfile around lines 5-16, switch the base image to
nvidia/cuda:12.1.0-devel-ubuntu20.04 and remove the ubuntu-toolchain
PPA/add-apt-repository and the update-alternatives calls: run apt-get update
then apt-get install the required packages (wget curl libtinfo-dev zlib1g-dev
libssl-dev build-essential libedit-dev libxml2-dev git gcc-9 g++-9) directly
(Ubuntu 20.04 provides GCC-9), drop the add-apt-repository and all
update-alternatives lines that force gcc/g++ symlinks, and keep a sanity check
like gcc --version; g++ --version after installation.
| 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 | ||
| rm Miniconda3-latest-Linux-x86_64.sh; |
There was a problem hiding this comment.
Verify Miniconda integrity.
Currently the installer isn’t verified. Above diff adds a sha256 check.
🤖 Prompt for AI Agents
In maint/scripts/pypi.Dockerfile around lines 17 to 19, the Miniconda installer
is downloaded and executed without verifying integrity; update the Dockerfile to
fetch or embed the expected SHA256 hash for the Miniconda installer, compute the
downloaded file's SHA256 (e.g. using sha256sum), compare it against the expected
value and abort the build if they differ, and only then run the installer;
ensure you still remove the installer file afterward and use robust curl flags
(fail/redirect) so failures are detected.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pyproject.toml (1)
2-11: Trim build-system.requires; gate patchelf to Linux.
- "build" is a front-end and unnecessary in PEP 517 requires.
- patchelf in build env can break on non-Linux; gate with a marker or drop if unused.
Apply:
[build-system] requires = [ - "build", "cmake>=3.26", "packaging", "setuptools>=61", "wheel", - "patchelf", + "patchelf; platform_system == \"Linux\"", "Cython>=3.0.0", ]Also confirm cmake>=3.26 is truly required; if not, relax to the minimum that passes CI to improve portability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(1 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). (3)
- GitHub Check: build-test-amd
- GitHub Check: Cursor Bugbot
- GitHub Check: format-check
| "patchelf", | ||
| "ninja", | ||
| "Cython", | ||
| "Cython>=3.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
Cython 3 floor: verify migration and avoid runtime coupling.
- Ensure code is Cython 3–clean (language_level=3, no deprecated 0.x/2.x syntax).
- Keep Cython only in build env; it should not be a runtime dependency.
Run to confirm no runtime pin and check for language_level usage:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Cython must NOT be in runtime reqs
rg -nP '^(Cython|cython)\b' requirements*.txt || true
# 2) Ensure cythonize or language_level is set somewhere
rg -nP '(cythonize\(|language_level\s*=)' -n -S -g '!**/build/**' || true
# 3) Inventory .pyx/.pxd files to scope migration risk
fd -t f -e pyx -e pxd -H || trueLength of output: 421
Remove Cython from runtime requirements; make it build-only and enforce Cython‑3.
- Cython is listed as a runtime dependency: requirements.txt:2 (Cython>=3.0.0) — remove from runtime requirements and keep it only in build deps (requirements-build.txt:2 or pyproject.build-system.requires).
- Cython sources exist: tilelang/jit/adapter/cython/cython_wrapper.pyx — ensure all .pyx/.pxd are compiled at build time and set language_level=3 (via cythonize(..., language_level=3) or setup.cfg/pyproject).
- Verify there are no runtime imports of Cython and add an explicit language_level=3 if missing.
🤖 Prompt for AI Agents
In pyproject.toml around line 9, Cython is incorrectly listed as a runtime
dependency; remove "Cython>=3.0.0" from runtime deps and instead add
Cython>=3.0.0 to build-system.requires (or requirements-build.txt) so it is
build-only; ensure all .pyx/.pxd files under tilelang/jit/adapter/cython are
compiled during package build (use cythonize in setup.py/pyproject build backend
or include build steps in pyproject) and explicitly set language_level=3 (via
cythonize(..., language_level=3) or in setup.cfg/pyproject Cython config); audit
code for any runtime imports of Cython and replace them with imports of the
compiled extensions or guard so there are no runtime Cython dependencies.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CMakeLists.txt (1)
9-13: Clarify/gate the status message with the actual toolchain checkThis prints even when the flags won’t be applied (non-GNU toolchains). Consider gating the message with the same compiler condition or clarifying it in the text.
Apply:
-if(TILE_LANG_STATIC_STDCPP) - message(STATUS "Enabling static linking of C++ standard library") +if(TILE_LANG_STATIC_STDCPP) + if(CMAKE_CXX_COMPILER_ID MATCHES "GNU") + message(STATUS "Enabling static linking of C++ standard library (GNU only)") + else() + message(STATUS "TILE_LANG_STATIC_STDCPP requested, but skipped for non-GNU toolchains") + endif() # Note: We'll apply static linking flags selectively to avoid Python extension conflicts # The flags will be applied per-target below rather than globally endif()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CMakeLists.txt(2 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). (4)
- GitHub Check: Cursor Bugbot
- GitHub Check: format-check
- GitHub Check: bot-task
- GitHub Check: format-check
🔇 Additional comments (1)
CMakeLists.txt (1)
229-233: Expose static libstdc++/libgcc to consumers via INTERFACE + GNU generator exprSTATIC targets have no link step; PRIVATE target_link_options are dropped. Expose the flags via INTERFACE and gate them to GNU so only consumers get static-libstdc++/libgcc (avoids Python extension conflicts).
File: CMakeLists.txt Lines: 229-233
-# Apply static linking flags only to static library to avoid Python extension conflicts -if(TILE_LANG_STATIC_STDCPP AND CMAKE_CXX_COMPILER_ID MATCHES "GNU") - target_link_options(tilelang_static PRIVATE -static-libstdc++ -static-libgcc) -endif() +# Apply static libstdc++/libgcc to consumers of the static lib (static libs have no link step) +if(TILE_LANG_STATIC_STDCPP) + target_link_options(tilelang_static INTERFACE + $<$<CXX_COMPILER_ID:GNU>:-static-libstdc++ -static-libgcc> + ) +endif()Verification: I could not run the supplied verify script in the environment (cmake: command not found). Run the verify script locally to confirm the link line for a consumer executable includes -static-libstdc++ and -static-libgcc on GNU.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CMakeLists.txt (1)
252-256: install() uses LIBRARY for a STATIC target; and tries to install IMPORTED tvm_runtime
- Static libs must be installed with ARCHIVE, not LIBRARY.
- When TVM_PREBUILD_PATH is set, tvm_runtime is IMPORTED; installing it will fail. Guard it to only install when built from source.
Apply:
-if(TILE_LANG_INSTALL_STATIC_LIB) - install(TARGETS tilelang_static tvm_runtime - LIBRARY DESTINATION lib${LIB_SUFFIX} - ) +if(TILE_LANG_INSTALL_STATIC_LIB) + install(TARGETS tilelang_static + ARCHIVE DESTINATION lib${LIB_SUFFIX} + ) + if(NOT DEFINED TVM_PREBUILD_PATH) + install(TARGETS tvm_runtime + RUNTIME DESTINATION bin + LIBRARY DESTINATION lib${LIB_SUFFIX} + ) + endif()
🧹 Nitpick comments (2)
CMakeLists.txt (2)
8-9: Option semantics: clarify/install vs build couplingTILE_LANG_INSTALL_STATIC_LIB also toggles BUILD_STATIC_RUNTIME (build behavior). Consider decoupling with a separate TILE_LANG_BUILD_STATIC_RUNTIME option or document the coupling explicitly.
10-14: Status message may misleadWe print “Enabling static linking” but no global/static effect occurs; only per-target logic applies later. After fixes below, ensure the message reflects what’s actually happening (and on which targets/toolchains).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CMakeLists.txt(2 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). (1)
- GitHub Check: Cursor Bugbot
| # Apply static linking flags only to static library to avoid Python extension conflicts | ||
| if(TILE_LANG_STATIC_STDCPP AND CMAKE_CXX_COMPILER_ID MATCHES "GNU") | ||
| target_link_options(tilelang_static PRIVATE -static-libstdc++ -static-libgcc) | ||
| endif() | ||
|
|
There was a problem hiding this comment.
-static-libstdc++ on a STATIC library does nothing; use INTERFACE link options
target_link_options on a STATIC target won’t take effect at archive creation. Propagate to consumers via INTERFACE and scope by compiler.
Apply:
-# Apply static linking flags only to static library to avoid Python extension conflicts
-if(TILE_LANG_STATIC_STDCPP AND CMAKE_CXX_COMPILER_ID MATCHES "GNU")
- target_link_options(tilelang_static PRIVATE -static-libstdc++ -static-libgcc)
-endif()
+# Apply flags as INTERFACE so consumers link libstdc++ statically; limit by compiler
+if(TILE_LANG_STATIC_STDCPP)
+ target_link_options(tilelang_static INTERFACE
+ "$<$<CXX_COMPILER_ID:GNU>:-static-libstdc++;-static-libgcc>"
+ "$<$<CXX_COMPILER_ID:Clang>:-static-libstdc++>"
+ )
+endif()📝 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.
| # Apply static linking flags only to static library to avoid Python extension conflicts | |
| if(TILE_LANG_STATIC_STDCPP AND CMAKE_CXX_COMPILER_ID MATCHES "GNU") | |
| target_link_options(tilelang_static PRIVATE -static-libstdc++ -static-libgcc) | |
| endif() | |
| # Apply flags as INTERFACE so consumers link libstdc++ statically; limit by compiler | |
| if(TILE_LANG_STATIC_STDCPP) | |
| target_link_options(tilelang_static INTERFACE | |
| "$<$<CXX_COMPILER_ID:GNU>:-static-libstdc++;-static-libgcc>" | |
| "$<$<CXX_COMPILER_ID:Clang>:-static-libstdc++>" | |
| ) | |
| endif() |
🤖 Prompt for AI Agents
In CMakeLists.txt around lines 230-234, the current target_link_options on the
STATIC target has no effect; change it to propagate options to consumers by
adding the options as INTERFACE link options scoped to GNU only. Replace the
PRIVATE line with an INTERFACE target_link_options entry (or use generator
expressions) that applies -static-libstdc++ and -static-libgcc for GNU compilers
so consumers linking against tilelang_static receive those flags.
* bugfix * [Build] Update build dependencies and Dockerfile configuration - Updated `pyproject.toml` and `requirements-build.txt` to specify Cython version as `Cython>=3.0.0`. - Removed unnecessary dependencies from the build system. - Enhanced `pypi.Dockerfile` to install gcc-9 and g++-9, and added ninja-build for improved build performance. - Updated conda environment creation to include Python 3.9 to 3.12, while removing the Python 3.8 environment. * cmake fix * fix * fix
1 participant
Summary by CodeRabbit
Breaking Changes
New Features
Chores
Dependencies