[MACA] Expand GEMM regression benchmark shapes#106
Conversation
|
👋 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! 🚀 |
📝 WalkthroughWalkthroughAdds a GEMM regression harness: centralized benchmark config/cases, a JIT-compiled tiled GEMM kernel with optional K-pipelining, runner helpers that profile via CUPTI, and nine public regression entrypoints to run named (M,N,K) cases. ChangesGEMM Regression Benchmark
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/maca/gemm/regression_example_gemm.py (1)
94-107:⚠️ Potential issue | 🟡 MinorFix the incorrect refactor summary (functions/entrypoint still present)
examples/maca/gemm/regression_example_gemm.pystill containsregression_example_gemm_autotune(),regression_example_gemm_intrinsics(),regression_example_gemm(), and theif __name__ == "__main__": tilelang.testing.regression()block (lines 94-107), so the summary claiming they were removed is incorrect.Repo-wide search found no call sites to these maca functions outside this file (only their definitions), so clarify whether these wrappers/entrypoint are intended to remain for compatibility/manual use or should be removed.
🤖 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 `@examples/maca/gemm/regression_example_gemm.py` around lines 94 - 107, The file still defines unused wrapper entrypoints regression_example_gemm_autotune, regression_example_gemm_intrinsics, regression_example_gemm and the if __name__ == "__main__": tilelang.testing.regression() block; either remove these symbols and the module-level main block if they are truly unused, or keep them but mark as deprecated and update the PR summary to state they remain for compatibility/manual use—ensure you search for any callers before deleting and update the file-level comment/README and the PR description accordingly; reference the exact names regression_example_gemm_autotune, regression_example_gemm_intrinsics, regression_example_gemm, and the main entrypoint when making the change.
🤖 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.
Outside diff comments:
In `@examples/maca/gemm/regression_example_gemm.py`:
- Around line 94-107: The file still defines unused wrapper entrypoints
regression_example_gemm_autotune, regression_example_gemm_intrinsics,
regression_example_gemm and the if __name__ == "__main__":
tilelang.testing.regression() block; either remove these symbols and the
module-level main block if they are truly unused, or keep them but mark as
deprecated and update the PR summary to state they remain for
compatibility/manual use—ensure you search for any callers before deleting and
update the file-level comment/README and the PR description accordingly;
reference the exact names regression_example_gemm_autotune,
regression_example_gemm_intrinsics, regression_example_gemm, and the main
entrypoint when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f491111f-5efe-4a32-bb34-5640eecea43e
📒 Files selected for processing (2)
examples/maca/gemm/regression_example_gemm.pytesting/maca/test_regression_example_gemm_shapes.py
VitalyAnkh
changed the title
# Conflicts: # examples/gemm/regression_example_gemm.py
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 `@examples/gemm/regression_example_gemm.py`:
- Around line 76-89: The wrapper functions
regression_bench_gemm_m1664_n1024_k262144,
regression_bench_gemm_m4096_n8192_k8192,
regression_bench_gemm_m4096_n8192_k28672, and
regression_bench_gemm_m8192_n1024_k8192 must not hard-code numeric indices into
_BENCH_GEMM_CASES; instead, locate the correct case by matching its "name" (the
same string used to build the wrapper name) against entries in _BENCH_GEMM_CASES
and call _process_bench_gemm_case(found_case); update each wrapper to search
_BENCH_GEMM_CASES for case["name"] == "<case name>" (or compute name
dynamically) and raise a clear error if not found so reordering of
_BENCH_GEMM_CASES cannot swap shapes silently.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 870c4aed-7f8c-4e58-bc5e-287a7b82b380
📒 Files selected for processing (1)
examples/gemm/regression_example_gemm.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
examples/gemm/regression_example_gemm.py (2)
81-85: 💤 Low valueConsider pre-building a lookup dict if _BENCH_GEMM_CASES grows.
The current O(n) linear search is fine for 9 cases, but if the benchmark suite expands significantly, converting
_BENCH_GEMM_CASESto a dict keyed by name would give O(1) lookup.♻️ Optional refactor to dict-based lookup
-_BENCH_GEMM_CASES = ( +_BENCH_GEMM_CASES = { + "bench_gemm_m1664_n1024_k262144": {"name": "bench_gemm_m1664_n1024_k262144", "M": 1664, "N": 1024, "K": 262144}, - {"name": "bench_gemm_m1664_n1024_k262144", "M": 1664, "N": 1024, "K": 262144}, # ... (repeat for all cases) -) +} def _get_bench_gemm_case(name): - for case in _BENCH_GEMM_CASES: - if case["name"] == name: - return case - raise KeyError(f"unknown GEMM benchmark case: {name}") + if name not in _BENCH_GEMM_CASES: + raise KeyError(f"unknown GEMM benchmark case: {name}") + return _BENCH_GEMM_CASES[name]🤖 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 `@examples/gemm/regression_example_gemm.py` around lines 81 - 85, The linear search in _get_bench_gemm_case over _BENCH_GEMM_CASES should be replaced with an O(1) lookup: build a dict mapping case["name"] to the case (e.g., _BENCH_GEMM_CASES_BY_NAME) at module load time and have _get_bench_gemm_case(name) return _BENCH_GEMM_CASES_BY_NAME[name] (raising KeyError if missing); update any code that mutates or constructs _BENCH_GEMM_CASES to keep the dict in sync or construct the dict from the list once to avoid repeat scans.
88-121: ⚖️ Poor tradeoffOptional: Derive case name from wrapper function name to reduce string repetition.
All nine wrappers follow the pattern
regression_bench_gemm_*→bench_gemm_*. You could eliminate the hard-coded case name strings by deriving them from the function's own__name__:def regression_bench_gemm_m1664_n1024_k262144(): import sys fn_name = sys._getframe().f_code.co_name case_name = fn_name.replace("regression_", "", 1) _process_bench_gemm_case(_get_bench_gemm_case(case_name))This eliminates typo risk and enforces the naming convention. However, the current explicit approach is more readable and grep-friendly, so this is purely optional.
🤖 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 `@examples/gemm/regression_example_gemm.py` around lines 88 - 121, Nine wrapper functions repeat the bench case string; change each regression_bench_gemm_* wrapper to derive the case name from the function's own name (fn_name = sys._getframe().f_code.co_name or equivalent) by stripping the "regression_" prefix and pass that derived case_name into _get_bench_gemm_case and then to _process_bench_gemm_case to remove hard-coded strings while keeping behavior of functions like regression_bench_gemm_m1664_n1024_k262144 and the helpers _get_bench_gemm_case/_process_bench_gemm_case unchanged.
🤖 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.
Nitpick comments:
In `@examples/gemm/regression_example_gemm.py`:
- Around line 81-85: The linear search in _get_bench_gemm_case over
_BENCH_GEMM_CASES should be replaced with an O(1) lookup: build a dict mapping
case["name"] to the case (e.g., _BENCH_GEMM_CASES_BY_NAME) at module load time
and have _get_bench_gemm_case(name) return _BENCH_GEMM_CASES_BY_NAME[name]
(raising KeyError if missing); update any code that mutates or constructs
_BENCH_GEMM_CASES to keep the dict in sync or construct the dict from the list
once to avoid repeat scans.
- Around line 88-121: Nine wrapper functions repeat the bench case string;
change each regression_bench_gemm_* wrapper to derive the case name from the
function's own name (fn_name = sys._getframe().f_code.co_name or equivalent) by
stripping the "regression_" prefix and pass that derived case_name into
_get_bench_gemm_case and then to _process_bench_gemm_case to remove hard-coded
strings while keeping behavior of functions like
regression_bench_gemm_m1664_n1024_k262144 and the helpers
_get_bench_gemm_case/_process_bench_gemm_case unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c73aecf-c6b1-4d6e-9ba9-04a647df1faa
📒 Files selected for processing (1)
examples/gemm/regression_example_gemm.py
1 participant
Summary
examples/gemm/regression_example_gemm.pybench_gemm_m1664_n1024_k262144bench_gemm_m4096_n1024_k8192bench_gemm_m4096_n8192_k8192bench_gemm_m4096_n28672_k8192bench_gemm_m4096_n8192_k28672bench_gemm_m8192_n1024_k8192bench_gemm_m8192_n8192_k8192bench_gemm_m8192_n28672_k8192bench_gemm_m8192_n8192_k28672Validation
python -m py_compile examples/gemm/regression_example_gemm.pyruff check examples/gemm/regression_example_gemm.pyruff format --check examples/gemm/regression_example_gemm.pygit diff --check origin/dev...HEAD