[Cleanup] Replace root reproducers with CPU regression coverage - #2878
Conversation
…ile-ai#2875) Remove the root-level reproducer scripts and add a proper pytest under testing/python/cpu covering vectorized vec_type arithmetic (+ - * /) on the CPU C target: the generated source must reach a float4 expression, compile as C++, and execute with numerically correct results.
|
👋 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! 🚀 |
📝 WalkthroughWalkthroughThe standalone vector arithmetic reproducer was deleted. A CPU regression test now validates vectorized addition, subtraction, multiplication, and division through C code generation and compiled execution. ChangesCPU vector arithmetic regression
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 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 `@testing/python/cpu/test_tilelang_cpu_vec_type.py`:
- Around line 73-79: Update the tensor allocations in the vec_arith test around
a, b, add, sub, mul, and div to explicitly use dtype=torch.float32 and
device="cpu". Preserve the existing divisor offset and avoid changing PyTorch
process-wide defaults.
🪄 Autofix
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: 391a2223-c3b8-41c6-9464-e3336a43884c
📒 Files selected for processing (3)
reproducer_metal_address_space.pyreproducer_vec_type.pytesting/python/cpu/test_tilelang_cpu_vec_type.py
💤 Files with no reviewable changes (1)
- reproducer_vec_type.py
CodeRabbit review finding: torch.randn/rand/empty respected PyTorch's process-wide default dtype/device, so another test changing set_default_dtype/set_default_device could create float64 or non-CPU tensors and break the float32 CPU kernel contract. Pin all six tensors explicitly without changing process-wide defaults.
2 participants
Problem
PR #2768 and PR #2770 added
reproducer_vec_type.pyandreproducer_metal_address_space.pyat the repository root while addressing twofailures encountered during the initial Apple M2 bring-up.
Those files do not belong at the repository root:
reproducer_metal_address_space.pyis empty and provides no regressioncoverage.
reproducer_vec_type.pyis a standalone executable script rather than apytest test under
testing/python.vec_typesupport for addition, subtraction, multiplication, and division.Issue #2875 correctly points out that regression coverage should live in the
project's test hierarchy instead of leaving standalone reproducers in the root
directory.
Root-cause trace
The CPU failure covered by
reproducer_vec_type.pyfollows this path:On arm64 targets, the generated source contains expressions such as:
Before PR #2768,
vec_type<T, N>did not define the required binary arithmeticoperators. The generated C++ therefore failed to compile with errors such as:
The implementation from PR #2768 is already present in
src/tl_templates/cpp/common.h. The remaining problem here is the location andcompleteness of its regression coverage, not another implementation defect.
The Metal file is a separate case. It is empty, so deleting it removes no test
coverage. The original PR #2770 Metal workaround is being superseded by the
type-driven codegen fix and dedicated Metal tests in PR #2824.
Fix
reproducer_metal_address_space.py.reproducer_vec_type.py.testing/python/cpu/test_tilelang_cpu_vec_type.pyas the permanent CPUregression test for PR [BugFix] Add arithmetic operators to vec_type in common.h for CPU codegen #2768.
+,-,*, and/through four actualT.Parallelloops.float4C-codegen path and emitsall four binary operators between vector operands.
execute it, and compare all four outputs with Torch references.
correctness rather than exceptional floating-point behavior.
This PR intentionally does not change
common.h, Metal codegen, runtime code,or public APIs. It preserves the implementation merged in PR #2768 and replaces
only the temporary root-level artifacts with repository-integrated coverage.
Relationship to the earlier PRs
vec_typearithmetic. This PR addscomplete, permanent CPU regression coverage for its
+,-,*, and/behavior.
Metal code is changed by this cleanup.
type- and storage-scope-driven Metal codegen and adds the corresponding Metal
regression tests under
testing/python/metal.The ownership is therefore disjoint:
vec_typearithmeticsrc/tl_templates/cpp/common.hfrom PR #2768testing/python/cpu/test_tilelang_cpu_vec_type.pyin this PRtesting/python/metal/test_metal_address_space.pyin PR #2824Tests
Run on a MacBook Air with Apple M2, macOS 15.6.1, arm64:
The same new test file was also run against the TileLang 0.1.12 wheel to verify
that it is sensitive to the original regression:
The source-inspection test still passes on 0.1.12 because lowering already
reaches
float4; it is the subsequent C++ compilation that correctly exposesthe missing operators. On the current implementation, lowering, compilation,
execution, and numerical validation all pass.
Scope
Three files are changed: 87 insertions and 37 deletions. There are no Metal
codegen changes, no
vec_typeimplementation changes, and no fallback orcompatibility path.
Summary
vec_typearithmetic operators through real vectorized CPU Ccodegen.
Closes #2875 (Issue).