[Enhancement] Disable data race check by default, opt-in via env var - #2851
Conversation
The VerifyParallelLoop data race check can report false positives on shared buffer stores whose per-thread addresses cannot be proven distinct (e.g. indices loaded from fragments). Disable it by default; users can opt in via the TILELANG_ENABLE_DATA_RACE_CHECK environment variable or by explicitly setting the tl.disable_data_race_check pass config to False.
|
👋 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! 🚀 |
📝 WalkthroughWalkthroughData-race checking is now disabled by default. ChangesData-race check configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/transform/verify_parallel_loop.cc`:
- Around line 143-146: Update the diagnostic text in the verify_parallel_loop
message to reference the valid PassConfigKey.TL_DISABLE_DATA_RACE_CHECK name and
explicitly instruct users to set it to True. Keep the existing
environment-variable guidance unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0790815f-ddd1-4da0-b202-1cfd5c1bf2b4
📒 Files selected for processing (4)
src/transform/verify_parallel_loop.cctesting/python/transform/test_tilelang_transform_verify_parallel_loop.pytilelang/backend/pass_pipeline/pipeline_utils.pytilelang/transform/pass_config.py
| os << "If you believe this is a false positive, disable the check by " | ||
| "setting `PassKey.TL_DISABLE_DATA_RACE_CHECK` in the pass config, " | ||
| "or by unsetting the `TILELANG_ENABLE_DATA_RACE_CHECK` environment " | ||
| "variable (the check is disabled by default)."; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the valid pass-config name and value.
Line 144 names PassKey.TL_DISABLE_DATA_RACE_CHECK, but tilelang/transform/pass_config.py defines PassConfigKey.TL_DISABLE_DATA_RACE_CHECK. The message also does not state that the key must be set to True.
Print the valid configuration key and value.
Proposed diagnostic fix
- "setting `PassKey.TL_DISABLE_DATA_RACE_CHECK` in the pass config, "
+ "setting `tl.disable_data_race_check` to `True` in the pass config, "📝 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.
| os << "If you believe this is a false positive, disable the check by " | |
| "setting `PassKey.TL_DISABLE_DATA_RACE_CHECK` in the pass config, " | |
| "or by unsetting the `TILELANG_ENABLE_DATA_RACE_CHECK` environment " | |
| "variable (the check is disabled by default)."; | |
| os << "If you believe this is a false positive, disable the check by " | |
| "setting `tl.disable_data_race_check` to `True` in the pass config, " | |
| "or by unsetting the `TILELANG_ENABLE_DATA_RACE_CHECK` environment " | |
| "variable (the check is disabled by default)."; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/transform/verify_parallel_loop.cc` around lines 143 - 146, Update the
diagnostic text in the verify_parallel_loop message to reference the valid
PassConfigKey.TL_DISABLE_DATA_RACE_CHECK name and explicitly instruct users to
set it to True. Keep the existing environment-variable guidance unchanged.
2 participants
The
VerifyParallelLoopdata race check can report false positives on shared buffer stores whose per-thread addresses cannot be proven distinct (e.g. indices loaded from fragments).Now disabled it by default; users can opt in via the
TILELANG_ENABLE_DATA_RACE_CHECKenvironment variable or by explicitly setting thetl.disable_data_race_checkpass config to False.Summary
VerifyParallelLoopdata-race checking by default.TILELANG_ENABLE_DATA_RACE_CHECK=1to enable the check.tl.disable_data_race_checkas a pass-context override.C++ style / lint notes
docs/developer_guide/cpp_style.md.