Skip to content

Conversation

@oraluben
Copy link
Collaborator

@oraluben oraluben commented Dec 21, 2025

Fix the problem that #1485 trying to notify in a more elegant way.

cc @kurisu6912

Summary by CodeRabbit

  • Chores
    • Z3 runtime library is now copied into the build output and platform-specific runtime lookup behavior was standardized for more reliable local builds.
    • Install/runtime-path settings consolidated across targets to simplify deployment and reduce unexpected linkage issues.
    • Dependency constraint for the TVM FFI tightened to the 0.1.x series while preserving a minimum required version.

✏️ Tip: You can customize this high-level summary in your review settings.

@oraluben oraluben requested a review from LeiWang1999 December 21, 2025 12:37
@github-actions
Copy link

👋 Hi! Thank you for contributing to the TileLang project.

Please remember to run pre-commit run --all-files in the root directory of the project to ensure your changes are properly linted and formatted. This will help ensure your contribution passes the format check.

We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 21, 2025

📝 Walkthrough

Walkthrough

CMakeLists.txt now installs the Z3 library into the build tvm directory when USE_Z3/USE_PYPI_Z3 are enabled, sets platform-specific BUILD_RPATH (@loader_path on macOS, $ORIGIN elsewhere), removes previous augmentation with Python Z3 paths and per-target rpath entries, and consolidates INSTALL_RPATH updates across public targets. Packaging files (pyproject.toml, requirements*.txt) unify the apache-tvm-ffi constraint into a single combined spec ~=0.1.0,>=0.1.6.

Changes

Cohort / File(s) Change Summary
Z3 library & RPATH behavior
CMakeLists.txt
Copy z3::libz3 into ${CMAKE_BINARY_DIR}/tvm when USE_Z3 && USE_PYPI_Z3; set BUILD_RPATH to @loader_path on Apple or $ORIGIN on other systems; removed legacy addition of Python z3 paths and per-target explicit BUILD_RPATH entries; consolidated INSTALL_RPATH for tilelang, tilelang_module, tvm, and tvm_runtime.
Install block / RPATH cleanup
CMakeLists.txt (install section)
Removed explicit per-target per-path RPATH entries in final install block; preserved unified multi-target INSTALL_RPATH approach.
Dependency spec updates
pyproject.toml, requirements-dev.txt, requirements.txt
Replaced separate/loose entries for apache-tvm-ffi with a single combined constraint apache-tvm-ffi~=0.1.0,>=0.1.6 across manifest and requirements files (narrows to 0.1.x while keeping minimum 0.1.6).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • LeiWang1999

Poem

🐰 I nudged a z3 into the build's warm den,
No more wandering through Python's glen;
On macOS branches, loaders softly call,
Elsewhere origins stand proud and tall —
A cozy hop, a library home for all.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is partially related to the changeset. It mentions 'isolated build' which relates to the BUILD_RPATH and dependency version constraint changes, but the phrasing is vague and doesn't clearly convey what 'the problem' is or what specific fix was applied. Consider a more specific title like 'Fix Z3 library path and TVM BUILD_RPATH for local isolated builds' to better communicate the core changes made.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b75cfb9 and e5d125d.

📒 Files selected for processing (3)
  • pyproject.toml
  • requirements-dev.txt
  • requirements.txt
⏰ 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). (5)
  • GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
  • GitHub Check: Build wheels for Python 3.9 on ubuntu-24.04-arm with CUDA-12.8
  • GitHub Check: Build wheels for Python 3.9 on ubuntu-latest with CUDA-12.8
  • GitHub Check: Build wheels for Python 3.9 on macos-latest with Metal
  • GitHub Check: Build SDist
🔇 Additional comments (3)
pyproject.toml (1)

30-31: Dependency consolidation is appropriate.

The comment clearly documents the reason for this version constraint (memory issue #1502), and the combined constraint ~=0.1.0,>=0.1.6 appropriately consolidates the requirement while restricting to the 0.1.x series. This aligns with changes in requirements.txt and requirements-dev.txt.

requirements-dev.txt (1)

3-3: Consistent version constraint update.

The version constraint matches the updates in requirements.txt and pyproject.toml, ensuring consistency across all dependency specifications for both isolated and non-isolated builds.

requirements.txt (1)

3-3: Version constraint is correctly specified.

The combined constraint ~=0.1.0,>=0.1.6 correctly restricts apache-tvm-ffi to versions >=0.1.6 and <0.2.0, aligning with the memory issue noted in pyproject.toml. The constraint is consistently applied across requirements.txt, requirements-dev.txt, and pyproject.toml, and version 0.1.6 is available on PyPI.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
CMakeLists.txt (1)

326-332: Optional: Simplify redundant conditional check.

The if(USE_Z3 AND USE_PYPI_Z3) check on line 326 is redundant if the previous fix is applied, since this entire block would already be inside the same conditional. The BUILD_RPATH logic itself is correct.

🔎 Suggested simplification (if previous fix applied)
 if(USE_Z3 AND USE_PYPI_Z3)
   # Copy libz3 library to build folder to workaround isolated build env issue
   get_target_property(Z3_LIBRARY_PATH z3::libz3 IMPORTED_LOCATION)
   file(COPY "${Z3_LIBRARY_PATH}" DESTINATION "${CMAKE_BINARY_DIR}/tvm")
   
-  if(USE_Z3 AND USE_PYPI_Z3)
-    if(APPLE)
-      set_target_properties(tvm PROPERTIES BUILD_RPATH "@loader_path")
-    else()
-      set_target_properties(tvm PROPERTIES BUILD_RPATH "\$ORIGIN")
-    endif()
+  if(APPLE)
+    set_target_properties(tvm PROPERTIES BUILD_RPATH "@loader_path")
+  else()
+    set_target_properties(tvm PROPERTIES BUILD_RPATH "\$ORIGIN")
   endif()
 endif()
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a874e4e and 8d05316.

📒 Files selected for processing (1)
  • CMakeLists.txt (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). (7)
  • GitHub Check: Test for Python 3.12 with Nightly-ROCm-7.1 (on self-hosted-amd)
  • GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
  • GitHub Check: Test for Python 3.12 with CUDA-12.8 (on self-hosted-nvidia)
  • GitHub Check: Build wheels for Python 3.9 on macos-latest with Metal
  • GitHub Check: Build wheels for Python 3.9 on ubuntu-24.04-arm with CUDA-12.8
  • GitHub Check: Build wheels for Python 3.9 on ubuntu-latest with CUDA-12.8
  • GitHub Check: Build SDist

@kurisu6912
Copy link
Collaborator

Thanks @oraluben, concise and effective!

I'll check the cutedsl CI

@oraluben
Copy link
Collaborator Author

Could not reproduce the test failure on my local H20, both with wheel installed / with local libraries from ./build/. The error seems unrelated (or related to the gcc on runner?)

@LeiWang1999 LeiWang1999 merged commit 0c3d913 into tile-ai:main Dec 25, 2025
11 checks passed
@oraluben oraluben deleted the local-build-z3 branch December 25, 2025 03:06
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.

3 participants