Skip to content

Conversation

@LeiWang1999
Copy link
Member

@LeiWang1999 LeiWang1999 commented Dec 23, 2025

as title.

Summary by CodeRabbit

  • Chores
    • Version updated to 0.1.7.post1.
    • Removed deprecated TL_DISABLE_FAST_MATH flag; use TL_ENABLE_FAST_MATH to control fast-math behavior.

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

@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 23, 2025

📝 Walkthrough

Walkthrough

This pull request removes deprecated fast-math configuration handling across the codebase. The TL_DISABLE_FAST_MATH enum key is deleted from the public API, conditional logic relying on both TL_DISABLE_FAST_MATH and TL_ENABLE_FAST_MATH is simplified to use only TL_ENABLE_FAST_MATH, and the version is bumped to 0.1.7.post1.

Changes

Cohort / File(s) Summary
Version bump
pyproject.toml (or equivalent)
Updated version string from 0.1.7 to 0.1.7.post1.
Public API configuration removal
tilelang/transform/pass_config.py
Removed TL_DISABLE_FAST_MATH enum member and its docstring from PassConfigKey. Consumers referencing the deleted key may need updates.
Conditional logic simplification
tilelang/engine/lower.py, tilelang/jit/adapter/libgen.py
Removed deprecated_warning import and dual-flag branches. enable_fast_math is now read directly from TL_ENABLE_FAST_MATH (defaults to False); inversion and warning paths eliminated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I’m a rabbit in the code, nibbling flags away,
TL_DISABLE hops off, TL_ENABLE gets its day.
Version patched, a tiny cheer,
Post-release carrot — bright and clear.
🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title correctly describes the main change: bumping the version to 0.1.7.post1. However, the changeset includes significant additional modifications beyond the version bump, specifically the removal of the deprecated TL_DISABLE_FAST_MATH configuration flag and related conditional logic across multiple files. The title should reflect all substantial changes in the PR. Consider updating it to describe both the version bump and the removal of the deprecated TL_DISABLE_FAST_MATH flag, e.g., '[Release] Bump version to 0.1.7.post1 and remove deprecated TL_DISABLE_FAST_MATH'.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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)
tilelang/engine/lower.py (1)

78-78: LGTM! Consistent removal of deprecated fast-math flag.

The simplification to use only TL_ENABLE_FAST_MATH aligns with the broader refactoring effort. The bool() wrapper provides explicit type conversion, though it's functionally equivalent to the approach in tilelang/jit/adapter/libgen.py (line 62) which omits the wrapper.

For consistency, consider whether both locations should use the same pattern (either both with or both without bool()), though this is a minor stylistic point.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 74aef5b and be9d174.

📒 Files selected for processing (4)
  • VERSION
  • tilelang/engine/lower.py
  • tilelang/jit/adapter/libgen.py
  • tilelang/transform/pass_config.py
💤 Files with no reviewable changes (1)
  • tilelang/transform/pass_config.py
⏰ 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: 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)
🔇 Additional comments (1)
tilelang/jit/adapter/libgen.py (1)

62-62: LGTM! Clean simplification of fast-math configuration.

The removal of dual-flag handling (TL_DISABLE_FAST_MATH / TL_ENABLE_FAST_MATH) in favor of a single TL_ENABLE_FAST_MATH flag simplifies the configuration logic. The default value of False is conservative and appropriate.

@@ -1 +1 @@
0.1.7
0.1.7.post1
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Searching for TL_DISABLE_FAST_MATH references ==="
rg -n "TL_DISABLE_FAST_MATH" --type=py 2>/dev/null || echo "No matches found"

echo -e "\n=== Checking git log for TL_DISABLE_FAST_MATH mentions ==="
git log --all --oneline --source --all | grep -i "TL_DISABLE_FAST_MATH\|fast_math\|deprecated" | head -20 2>/dev/null || echo "No git log matches"

echo -e "\n=== Searching for deprecation warnings related to fast_math ==="
rg -n -C3 "deprecat" --type=py | grep -i "fast\|math" 2>/dev/null || echo "No deprecation mentions found"

echo -e "\n=== Checking VERSION file history ==="
git log --all --oneline VERSION 2>/dev/null | head -10 || echo "Cannot access git history"

Repository: tile-ai/tilelang

Length of output: 1231


🏁 Script executed:

#!/bin/bash
echo "=== Check PassConfigKey enum for TL_DISABLE_FAST_MATH ==="
rg -n "PassConfigKey|TL_DISABLE_FAST_MATH" --type=py -A2 | head -50

echo -e "\n=== Check git diff for recent VERSION bump ==="
git show be9d174 --stat

echo -e "\n=== Check recent ENABLE_FAST_MATH refactor ==="
git show e7e3835 --stat

Repository: tile-ai/tilelang

Length of output: 50372


🏁 Script executed:

#!/bin/bash
echo "=== Check the ENABLE_FAST_MATH refactor commit ==="
git show e7e3835 --name-only | head -30

echo -e "\n=== Check if TL_DISABLE_FAST_MATH enum value exists ==="
git show e7e3835:tilelang/transform/pass_config.py 2>/dev/null | grep -A5 -B5 "DISABLE_FAST_MATH\|ENABLE_FAST_MATH" || echo "Check previous version"

echo -e "\n=== Check current pass_config.py for these keys ==="
cat tilelang/transform/pass_config.py | grep -A2 -B2 "FAST_MATH\|ENABLE_FAST_MATH"

Repository: tile-ai/tilelang

Length of output: 2743


Post-release versioning is inappropriate for removing public API.

According to semantic versioning best practices, deprecated APIs should first be marked in a minor release with warnings, and removal should only occur in a subsequent major release after users have time to migrate. The version bump to 0.1.7.post1 violates this practice.

Git history shows that TL_DISABLE_FAST_MATH was deprecated in commit e7e3835 (marked "will be deprecated in the 0.1.7 release"), but it has been completely removed from the PassConfigKey enum in this post-release. This is a breaking change because users still actively using TL_DISABLE_FAST_MATH (as seen in examples/deepseek_v32/inference/kernel.py) will encounter errors.

Post-release versions (.post1) are reserved for packaging or distribution fixes without functional changes. Removing a public configuration key warrants at least a minor version bump (e.g., 0.2.0). If the deprecation was properly introduced in 0.1.7 with runtime warnings, a subsequent minor or major release should have been used for the actual removal.

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

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be9d174 and 1dd1145.

📒 Files selected for processing (1)
  • tilelang/engine/lower.py
🧰 Additional context used
🧬 Code graph analysis (1)
tilelang/engine/lower.py (1)
tilelang/transform/pass_config.py (1)
  • PassConfigKey (6-157)

enable_fast_math = not disable_fast_math
else:
enable_fast_math = bool(cfg.get(PassConfigKey.TL_ENABLE_FAST_MATH, False))
enable_fast_math = bool(cfg.get(PassConfigKey.TL_ENABLE_FAST_MATH, False))
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for any remaining references to TL_DISABLE_FAST_MATH in the codebase
echo "=== Searching for TL_DISABLE_FAST_MATH references ==="
rg -n 'TL_DISABLE_FAST_MATH' --type=py
echo ""
echo "=== Searching in documentation ==="
rg -n 'TL_DISABLE_FAST_MATH' --type=md
echo ""
echo "=== Searching in all file types (case-insensitive) ==="
rg -in 'disable.*fast.*math' --max-count=20

Repository: tile-ai/tilelang

Length of output: 575


🏁 Script executed:

#!/bin/bash
# Verify the context around line 78 in tilelang/engine/lower.py
echo "=== Context around line 78 in lower.py ==="
sed -n '70,110p' tilelang/engine/lower.py | cat -n

Repository: tile-ai/tilelang

Length of output: 1927


🏁 Script executed:

#!/bin/bash
# Check if TL_ENABLE_FAST_MATH is properly defined/used elsewhere
echo "=== Searching for TL_ENABLE_FAST_MATH usage ==="
rg -n 'TL_ENABLE_FAST_MATH' --type=py

Repository: tile-ai/tilelang

Length of output: 10638


Critical issue: Incomplete deprecation of TL_DISABLE_FAST_MATH.

While the code change at line 78 is technically correct in using only TL_ENABLE_FAST_MATH with a default of False, the deprecation of TL_DISABLE_FAST_MATH is incomplete. The deprecated flag still exists and is actively used in the codebase:

  • examples/deepseek_v32/inference/kernel.py:11 still uses TL_DISABLE_FAST_MATH: True
  • src/op/builtin.cc:27 and src/op/builtin.h:47 still define the C++ option

Breaking change: Code using the old TL_DISABLE_FAST_MATH flag will now silently fail because line 78 no longer reads this flag. This requires either:

  1. Updating all usages to TL_ENABLE_FAST_MATH, or
  2. Implementing a deprecation migration path that honors the old flag temporarily while warning users

@LeiWang1999 LeiWang1999 merged commit 3c11823 into tile-ai:main Dec 24, 2025
9 of 10 checks passed
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