Skip to content

fix(rocke): make f32 tanh AMDGPU-lowerable - #11697

Open
tenpercent wants to merge 3 commits into
developfrom
users/mpodkory/rocke/aick-2054
Open

tenpercent wants to merge 3 commits into
developfrom
users/mpodkory/rocke/aick-2054

Conversation

@tenpercent

@tenpercent tenpercent commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Motivation

rocKE lowered math.tanh to llvm.tanh.f32, which the AMDGPU production path cannot lower without a device libcall. The existing activation helper avoided that intrinsic but used a positive-exponent formulation that could overflow for large positive inputs.

ISSUE ID : AICK-2054

Technical Details

  • Restrict the core math.tanh operation to f32 and report matching diagnostics from the Python and C++ builders and verifiers for unsupported narrow operands.
  • Replace llvm.tanh.f32 emission in both engines with a piecewise f32 implementation:
    • a polynomial for abs(x) < 0.625;
    • an exp2 and reciprocal formulation outside that range;
    • bitwise sign restoration to preserve negative zero.
  • Route the shared activation helper through the canonical operation so elementwise, fused activation, and softcap consumers use the stable lowering.
  • Remove the unused llvm.tanh.f32 declaration.
  • Add Python and C++ lowering-contract tests, host numeric coverage, elementwise parity cases, GPU numeric configurations, updated LLVM golden hashes, and documentation.

Test Plan

  • Focused Python suite: 9 passed, 4 subtests passed.
  • Focused C++ lowering test: 1/1 passed.
  • Elementwise Python/C++ byte identity: 10/10 configurations under LLVM 20, 22, and 23.
  • Host numeric coverage for signed zero, infinities, NaN, tiny inputs, the piecewise boundary, and the measured f32 error bound.
  • Focused LLVM compiled through ROCm 7.14 COMGR.
  • Pre-commit range checks.
  • Direct f32 and f16/bf16 elementwise numeric execution on gfx942 and gfx950: 3/3 cases passed per target.
  • Additional device f32 sweep over 2,000,001 points on [-20, 20] passed on both targets.

Test Result

GPU validation used commit 204dee102d6da54663ba989765d8e8aef4825c42, modules ubuntu-24 and rocm/7.14.0, LLVM 23, and PyTorch 2.13.0+rocm7.14.0.

Target Slurm job Direct f32 and f16/bf16 elementwise Dense f32 sweep maximum absolute error
gfx942 67921162 3/3 passed 5.960464477539063e-08
gfx950 67921163 3/3 passed 5.960464477539063e-08

Both jobs completed with exit status 0. The existing numeric cases cover signed zero, infinities, NaN classification, tiny-value bit identity, and the piecewise boundary. The additional 2,000,001-point device sweep on [-20, 20] compared results against NumPy float64 tanh rounded to float32. Its observed maximum absolute error was below one f32 epsilon on both targets; this is a measured bound for that probe, not an exhaustive bound over all f32 inputs. The focused host tests also remain within one f32 epsilon over their deterministic probe.

These runs compiled Python-generated LLVM through COMGR, loaded the resulting HSACO through HIP, and executed the rocKE kernels on the GPU. References were computed on CPU using NumPy/torch. A separately hashed validation adapter isolated COMGR in a fresh subprocess, supplied the verified ROCm release to the version guard because the wheel library tree lacks version metadata, retained LLVM/HSACO artifacts, and added the dense device sweep; PR source was unchanged. Python/C++ emission parity was validated separately as listed above.

The three GPU cases are registered in the differential numeric harness, which the examples runner invokes for gfx950. The main platform/tests/run_all.py pytest path does not invoke that harness; adding a GPU pytest wrapper remains a separate follow-up. The additional dense device sweep is validation-adapter coverage, not part of the PR's routine suite.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Sep 4, 2026

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ✅ Pass
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled
🤖 therock-pr-bot ✅ Pass

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Sep 4, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@tenpercent
tenpercent marked this pull request as ready for review September 10, 2026 18:31
@tenpercent
tenpercent requested review from a team as code owners September 10, 2026 18:31
@AviralGoelAMD
AviralGoelAMD requested a lite review from Copilot September 16, 2026 03:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@tenpercent

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

…ke/aick-2054

# Conflicts:
#	dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/core/lower_llvm.py
#	dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/differential/golden/llvm_gfx_all.json

Co-authored-by: tenpercent <4273004+tenpercent@users.noreply.github.com>

Copilot AI commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved the conflicts by merging the latest develop and preserving both branches’ rocKE lowering and golden-baseline changes in merge commit 45be084a0. The focused tanh suite passes (9 tests, 4 subtests).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants