[Feat] Improve T.reduce_absmax to use less abs call

[kurisu6912]

Summary by CodeRabbit

• Refactor
  • Internal reduction operation logic updated to use a consistent accumulator variable and dtype handling across arithmetic and bitwise reductions, improving clarity and maintainability.
  • No externally visible API changes; behavior remains the same for end users.

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

[github-actions]

👋 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]

📝 Walkthrough

Walkthrough

The MakeReduce parameter in src/op/reduce.cc and its declaration in src/op/reduce.h is renamed from lhs to acc; internal casts and all arithmetic/bitwise/reduction usages are updated to operate on acc consistently.

Changes

Cohort / File(s)	Summary
Reduce accumulator rename & usage src/op/reduce.cc, src/op/reduce.h	Renamed MakeReduce parameter from lhs to acc (signature change in header and definition). Replaced all internal references and dtype-casting to use acc for arithmetic, Min/Max, and bitwise operations.

Sequence Diagram(s)

(Skipped — changes are a parameter rename and internal reference updates, not a new multi-component control flow.)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• [Language] Refactor reduce and support shared memory as its in/out #1219: Modifies MakeReduce in src/op/reduce.cc as well (changes around AbsMax computation and reduction logic), likely overlapping the same code paths.

Poem

🐰 I nibbled code with careful tact,
I found a name that better tracks,
lhs hopped off, acc took place,
Now reductions wear a clearer face,
A tiny hop, a tidy patch! 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title references improving T.reduce_absmax to use fewer abs calls, but the actual changes are parameter renames from 'lhs' to 'acc' in MakeReduce methods, with no evidence of abs call optimization.	Update the PR title to accurately reflect the actual changes: parameter renaming for clarity (e.g., 'Rename MakeReduce parameter from lhs to acc for clarity'). Or verify the implementation changes match the stated objective of reducing abs calls.
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.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.

_{✏️ Tip: You can configure your own custom Pre-merge Checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4a50e5a and 8d7c52d.

📒 Files selected for processing (1)

• src/op/reduce.h

🧰 Additional context used

🧬 Code graph analysis (1)

src/op/reduce.h (1)

src/op/reduce.cc (2)

• MakeReduce (102-127)
• MakeReduce (102-103)

🔇 Additional comments (1)

src/op/reduce.h (1)

118-118: LGTM! The parameter rename improves clarity.

Renaming lhs to acc better reflects the accumulator pattern used in reduction operations. The parameter holds the running accumulated value across iterations, making acc semantically more appropriate than lhs (left-hand side).

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @src/op/reduce.cc:
- Around line 102-103: The header declaration for ReduceOpNode::MakeReduce
currently names the first parameter `a` while the implementation uses `acc`;
update the declaration of PrimExpr ReduceOpNode::MakeReduce to use `const
PrimExpr &acc` (i.e., change `a` → `acc`) so the parameter name matches the
implementation and signature remains otherwise unchanged.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c8f4e23 and 4a50e5a.

📒 Files selected for processing (1)

• src/op/reduce.cc

🧰 Additional context used

🧬 Code graph analysis (1)

src/op/reduce.cc (2)

tilelang/language/tir/ir.pyi (1)

• Cast (6-6)

tilelang/language/tir/op.py (1)

• abs (2504-2520)

⏰ 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). (1)

• GitHub Check: Test for Python 3.12 with Metal (on macos-latest)

🔇 Additional comments (2)

src/op/reduce.cc (2)

105-127: LGTM - Consistent variable rename throughout function body.

All references to lhs have been correctly updated to acc, maintaining the logic of each reduction type.

---

102-127: Verify alignment between PR title and implementation.

The PR title mentions "Improve T.reduce_absmax to use less abs call", but the code changes only show a variable rename from lhs to acc. Line 117 still contains tvm::abs(rhs), and there's no visible reduction in the number of abs calls.

Please confirm whether:

1. This PR only addresses the variable rename for clarity, or
2. Additional optimization to reduce abs calls is intended but not yet implemented