⚡ Thunderbolt: Softmax — 8x Unrolling Optimization#83
Conversation
Adds aggressive 8x unrolling to hide max and exp calculation latencies. Co-authored-by: bugparty <1510776+bugparty@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughAdds the AVX2 ChangesSoftmax v6
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ml_kernels/src/test_naive_ops.cpp (1)
184-214: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest doesn't exercise
softmax_v6's scalar tail path.Input length is 72 (=64+8), so
n % 8 == 0andn % 64 == 8; the single-element scalar tail loops insoftmax_v6(max/sum/normalize) never execute for this test. Consider using a length not divisible by 8 (e.g. drop or add one element) to cover the scalar fallback path.🤖 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 `@ml_kernels/src/test_naive_ops.cpp` around lines 184 - 214, Update test_softmax_v6 so its input length is not divisible by 8, ensuring softmax_v6 executes its scalar tail handling in the max, sum, and normalization passes. Adjust the test data or add a single element while preserving the existing naive-versus-v6 comparison and sum-to-one assertions.
🤖 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 `@ml_kernels/src/test_naive_ops.cpp`:
- Around line 184-214: Update test_softmax_v6 so its input length is not
divisible by 8, ensuring softmax_v6 executes its scalar tail handling in the
max, sum, and normalization passes. Adjust the test data or add a single element
while preserving the existing naive-versus-v6 comparison and sum-to-one
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 772ea217-f65d-4003-ae8d-cd11b814c483
📒 Files selected for processing (4)
.jules/thunderbolt.mdml_kernels/include/ml_kernels/softmax.hml_kernels/src/kernel_bench.cppml_kernels/src/test_naive_ops.cpp
💡 What: Aggressive 8x unrolling across all map-reduce phases of the AVX2 softmax kernel (
softmax_v6), processing 64 elements per iteration.🎯 Why: Complex mathematical sequences like the
_mm256_max_psand multi-instructionexp256approximations exhibit FMA latencies that can bottleneck 4x unrolled implementations. 8x unrolling effectively hides this latency by maintaining 8 independent accumulator chains without causing YMM register spilling.🏗️ How: 8x unrolling across the max, exp/sum, and normalization loops explicitly utilizing 8 unique YMM registers per section while keeping standard scalar fallbacks for the remainder.
📊 Impact: Showed up to ~5.56 GFLOP/s vs ~5.19 GFLOP/s (softmax_v5) on N=16384 arrays in fixed-memory benchmarking, confirming transition toward a throughput-bound behavior.
🖥️ Tested on: Intel(R) Xeon(R) Processor @ 2.30GHz, Linux, GCC 13.3.0.
🔬 How to reproduce:
cd build && make -j$(nproc)./ml_kernels/ml_kernel_bench --filter "softmax_v6"PR created automatically by Jules for task 1136076901946106662 started by @bugparty
Summary by CodeRabbit
New Features
softmax_v6AVX2 softmax implementation with enhanced parallel processing for improved throughput on large inputs.softmax_v6performance.Bug Fixes
Documentation