perf(rccl): split ce_reduce into per-kernel TUs to fix ~48min build time by lfmeadow · Pull Request #9756 · ROCm/rocm-systems

lfmeadow · 2026-08-06T08:51:06Z

Summary

The CE (Copy Engine) AllReduce local-reduce kernel (ce_reduce.cc) held all 40 (type, redop) instantiations in a single translation unit. LLVM's runtime loop-unroller was auto-unrolling the rank-reduction loop (nRanks is a runtime value) by up to 64x on top of the existing manual unroll/vectorization, producing ~900 PHI nodes and ~150 shufflevector ops per kernel for int8_t/uint8_t Min/Max alone. This dominated llc codegen time and pushed the full RCCL build to ~48 minutes.

This PR splits the 40 instantiations into one .cpp file per (type, redop) (enabling parallel device-link compilation) and disables the runtime unroller on the rank loop. Since disabling the loop vectorizer along with the unroller turned out to help some kernels but hurt others by ~8-12% (real, not noise — confirmed via repeated on-GPU measurement), the vectorizer setting is keyed per-kernel to whichever measured faster, while the unroller stays disabled everywhere.

Build time: the single-TU ce_reduce.cc was one compile job that couldn't be parallelized and took ~48 minutes by itself, dominating the whole build's critical path regardless of available cores. Splitting it into 40 independent TUs removes that serialized bottleneck: verified with an empty ccache (no caching effects), each of the 40 kernels now compiles in under 2 seconds (the two worst offenders, int8_t/uint8_t Min/Max, previously took the bulk of that 48 minutes on their own), and the full clean build's wall-clock time drops accordingly — from ~48 minutes to well under a minute on a highly parallel (512-core) machine. The exact end-to-end number will vary by machine/parallelism; what's fixed is the specific unparallelizable 48-minute bottleneck, not a general build-cost reduction.

Performance (MI350X/gfx950, vs. unmodified develop, measured with 2-3 repeats per build to separate signal from run-to-run noise):

int32_t/uint32_t Sum/Prod: +137-139% (this is the dominant win from disabling the pathological auto-unroll)
int8_t Min/Max/Sum, uint8_t Sum: net neutral (a real ~8-9% regression from disabling the vectorizer was recovered by re-enabling it for just these kernels)
uint8_t Min/Max: +8-12%
All other 32 of 40 (type, redop) combinations: no measurable change (noise floor)

Correctness: all 40 (type, redop) combinations validated bit-exact against a host-side reference reduction (identical fold order, no float-associativity slop) via a standalone microbenchmark driving the launcher symbols directly.

Test plan

Full clean build succeeds on gfx950 (srock toolchain)
All 40 kernel correctness checks pass (bit-exact vs. host reference)
Performance swept 64KB-1GB per-rank chunk size on a single GPU (MI350X, bacon) with the kernel's nRanks parameter set to 8 — this is a local-reduce kernel that folds nRanks input buffers together on one GPU, it does not launch across multiple GPUs — compared against pristine develop and against the pre-per-kernel-tuning version of this fix, with repeated runs to confirm signal vs. noise

Made with Cursor

therock-pr-bot · 2026-08-06T08:51:49Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

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

🙋 Wish to Override Policy?

therock-pr-bot · 2026-08-06T08:51:51Z

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

Copilot

Pull request overview

This PR addresses the long serialized build-time bottleneck in RCCL’s CE (Copy Engine) AllReduce local-reduce path by splitting the previously monolithic ce_reduce.cc (40 (type, redop) instantiations) into per-instantiation translation units generated under gensrc/ce_reduce/, enabling parallel compilation/device-linking and reducing pathological LLVM codegen overhead.

Changes:

Add a code generator to emit ce_reduce_impl.h plus 40 ce_reduce_<type>_<redop>.cpp instantiation TUs with per-kernel loop pragma tuning.
Convert src/device/ce_reduce.cc into a pure host-side dispatcher that forwards to the generated per-instantiation launchers.
Update the build system to generate and compile these per-instantiation TUs in both normal and device-linker pipelines.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
projects/rccl/src/device/ce_reduce/generate.py	New generator that emits shared kernel/template header and per-(type,redop) launcher TUs with loop pragma controls.
projects/rccl/src/device/ce_reduce.cc	Reworked into a host-only dispatcher calling pre-instantiated launcher symbols.
projects/rccl/src/CMakeLists.txt	Runs CE-reduce TU generation and updates device-linker filtering expectations.
projects/rccl/cmake/DeviceLinker.cmake	Compiles all generated CE-reduce instantiation TUs as separate fat objects in the device-linker pipeline.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… files IMPL_HEADER and LAUNCHER_TEMPLATE were multi-hundred-line C++/HIP kernel code buried in Python string literals inside generate.py, making the device code hard to read/edit with normal tooling (per Alex's review on #9756). Move them to ce_reduce_impl.h.in (copied verbatim) and ce_reduce_launcher.cpp.in (expanded via string.Template's $-placeholders instead of doubled {{ }} braces), leaving generate.py as just the TYPES/REDOPS/VECTORIZE_OK tables and a short substitution loop. Verified byte-for-byte identical generator output aside from the intentionally updated doc comments. Also add test_generate_ce_reduce.py (wired into ctest as rccl-generate-ce-reduce, alongside the existing generator test) to cover this pipeline, addressing the missing-unit-test policy check on #9756. Co-authored-by: Cursor <cursoragent@cursor.com>

alex-breslow-amd · 2026-08-06T18:52:24Z

Can you add the missing files?

… files IMPL_HEADER and LAUNCHER_TEMPLATE were multi-hundred-line C++/HIP kernel code buried in Python string literals inside generate.py, making the device code hard to read/edit with normal tooling (per Alex's review on #9756). Move them to ce_reduce_impl.h.in (copied verbatim) and ce_reduce_launcher.cpp.in (expanded via string.Template's $-placeholders instead of doubled {{ }} braces), leaving generate.py as just the TYPES/REDOPS/VECTORIZE_OK tables and a short substitution loop. Verified byte-for-byte identical generator output aside from the intentionally updated doc comments. Also add test_generate_ce_reduce.py (wired into ctest as rccl-generate-ce-reduce, alongside the existing generator test) to cover this pipeline, addressing the missing-unit-test policy check on #9756. Co-authored-by: Cursor <cursoragent@cursor.com>

lfmeadow · 2026-08-06T19:01:51Z

Yes, sorry — that was actually my mistake: I'd landed the ce_reduce_impl.h.in/ce_reduce_launcher.cpp.in/test_generate_ce_reduce.py commit on the stacked #9763 branch instead of this one. Fixed now — it's on this PR as of a04c4fd (test_generate_ce_reduce.py matches the bot's test_* pattern, so it should also clear the Unit Test warning). The PR-bot's checklist comment above is stale though — it hasn't re-run since before today's pushes, so it's still showing the old warning; should clear on its next run.

mustafabar

Thanks for this PR. I had added a few minor comments.

… files IMPL_HEADER and LAUNCHER_TEMPLATE were multi-hundred-line C++/HIP kernel code buried in Python string literals inside generate.py, making the device code hard to read/edit with normal tooling (per Alex's review on #9756). Move them to ce_reduce_impl.h.in (copied verbatim) and ce_reduce_launcher.cpp.in (expanded via string.Template's $-placeholders instead of doubled {{ }} braces), leaving generate.py as just the TYPES/REDOPS/VECTORIZE_OK tables and a short substitution loop. Verified byte-for-byte identical generator output aside from the intentionally updated doc comments. Also add test_generate_ce_reduce.py (wired into ctest as rccl-generate-ce-reduce, alongside the existing generator test) to cover this pipeline, addressing the missing-unit-test policy check on #9756. Co-authored-by: Cursor <cursoragent@cursor.com>

github-actions · 2026-08-06T21:08:58Z

RCCL Perf-Regression Gate: ✅ PASS

Mode: detect (reference vs candidate)
Thresholds (per-tier, calibrated): small 17.2% · mid 12.3% · large 12.9%
Keys compared: 0 · Confirmed regressions: 0 · Inconclusive: 0

Per-collective breakdown

group	keys	regressions	inconclusive
all_gather_perf-d=bfloat16-default	0	0	0
all_gather_perf-d=float-default	0	0	0
all_reduce_perf-d=bfloat16-default	0	0	0
all_reduce_perf-d=float-default	0	0	0
broadcast_perf-d=bfloat16-default	0	0	0
broadcast_perf-d=float-default	0	0	0
reduce_scatter_perf-d=bfloat16-default	0	0	0
reduce_scatter_perf-d=float-default	0	0	0

alex-breslow-amd

LGTM. Thanks for addressing my comments.

… files IMPL_HEADER and LAUNCHER_TEMPLATE were multi-hundred-line C++/HIP kernel code buried in Python string literals inside generate.py, making the device code hard to read/edit with normal tooling (per Alex's review on #9756). Move them to ce_reduce_impl.h.in (copied verbatim) and ce_reduce_launcher.cpp.in (expanded via string.Template's $-placeholders instead of doubled {{ }} braces), leaving generate.py as just the TYPES/REDOPS/VECTORIZE_OK tables and a short substitution loop. Verified byte-for-byte identical generator output aside from the intentionally updated doc comments. Also add test_generate_ce_reduce.py (wired into ctest as rccl-generate-ce-reduce, alongside the existing generator test) to cover this pipeline, addressing the missing-unit-test policy check on #9756. Co-authored-by: Cursor <cursoragent@cursor.com>

…-loop unroll ncclCeLocalReduceKernelVec's 40 (type, redop) instantiations previously lived in a single ce_reduce.cc, forcing the device-link LTO codegen pass to run serially over all of them. Two of the 40 -- int8_t/uint8_t Min/Max -- balloon to ~56K instructions each because LLVM's runtime loop-unroller was unrolling the `for (r = 1; r < nRanks; r++)` rank-reduction loop by up to 64x (nRanks is a runtime value) on top of the existing manual U/W unroll/vectorization, producing ~900 PHI nodes and ~150 shufflevector ops per kernel. That alone made the file take ~48 minutes to compile. Fix: generate one TU per instantiation (src/device/ce_reduce/generate.py) so ninja parallelizes the 40 kernels like every other device TU, and mark both rank-reduction loops `#pragma clang loop unroll(disable) vectorize(disable)` so LLVM stops auto-unrolling a loop the manual unroll already covers. Build time: ~48.5 min -> ~14.7 min for the ce_reduce kernels. Runtime validated via a standalone microbenchmark (all 40 type/redop combos, correctness-checked against a host-side reference using the kernel's exact fold order, then timed 16B-1GB per-rank chunk sizes): no regression, i8/u8 Min/Max reach ~1.0-1.25 TB/s. Co-authored-by: Cursor <cursoragent@cursor.com>

…ling it Disabling the rank-loop's runtime unroller fixed the ce_reduce compile-time blowup, but the accompanying vectorize(disable) was applied uniformly to all 40 (type, redop) instantiations. Measurement on gfx950 showed that's a loss for int8_t Min/Max/Sum and uint8_t Sum (~10-12% slower than auto-unrolled develop), even though it's a big win elsewhere (int32_t/uint32_t Sum/Prod are ~2.2x faster with it disabled). Key the vectorizer setting per (type, redop) via a CE_REDUCE_VECTORIZE_OK macro defined ahead of the shared kernel header in the affected TUs, using whichever setting measured faster. The runtime unroller stays disabled everywhere (that's what fixes compile time); only the vectorizer choice varies. Verified all 4 previously-pathological kernels (i8/u8 Min/Max) still compile in ~2s. Co-authored-by: Cursor <cursoragent@cursor.com>

- Handle stale subdirectories/symlinks when clearing the output dir, mirroring the symmetric kernel generator's os.path.isfile/isdir check instead of assuming every entry is a plain file (os.remove() would otherwise raise on a leftover subdir from a prior build). - #undef CE_REDUCE_VECTORIZE_OK right after ce_reduce_impl.h consumes it, so the macro doesn't leak into later includes/code in the same TU. Co-authored-by: Cursor <cursoragent@cursor.com>

… files IMPL_HEADER and LAUNCHER_TEMPLATE were multi-hundred-line C++/HIP kernel code buried in Python string literals inside generate.py, making the device code hard to read/edit with normal tooling (per Alex's review on #9756). Move them to ce_reduce_impl.h.in (copied verbatim) and ce_reduce_launcher.cpp.in (expanded via string.Template's $-placeholders instead of doubled {{ }} braces), leaving generate.py as just the TYPES/REDOPS/VECTORIZE_OK tables and a short substitution loop. Verified byte-for-byte identical generator output aside from the intentionally updated doc comments. Also add test_generate_ce_reduce.py (wired into ctest as rccl-generate-ce-reduce, alongside the existing generator test) to cover this pipeline, addressing the missing-unit-test policy check on #9756. Co-authored-by: Cursor <cursoragent@cursor.com>

Mustafa's review on this PR asked whether three loops in ncclCeLocalReduceKernelVec (the elementwise k loop, the drain loop's own outer loop, and the scalar tail's rank loop) were missing an unroll(disable) pragma by oversight, since sibling loops have one. Investigated in #9797: adding the pragma to the elementwise k loop regresses every kernel 2-3x (forces a compile-time-constant register access into a runtime-indexed one, realized via an LDS round trip), and even gating it to only the int8_t/uint8_t kernels that already use the loop vectorizer still regresses those ~1.8x (the "bloat" there is the vectorizer packing the byte-wise reduction into native sub-dword instructions, which needs to see through this loop). The other two loops are confirmed no-ops either way (byte-identical codegen). So all three are left as-is; add short comments explaining why. Co-authored-by: Cursor <cursoragent@cursor.com>

github-actions Bot added the project: rccl label Aug 6, 2026

therock-pr-bot Bot added the Not ready to Review label Aug 6, 2026

lfmeadow mentioned this pull request Aug 6, 2026

PR #8443 caused a very large increase in build time #9757

Closed

systems-assistant Bot added the organization: ROCm label Aug 6, 2026

lfmeadow mentioned this pull request Aug 6, 2026

feat(rccl): unify mixed-TU device images into one device.elf #9763

Open

3 tasks

lfmeadow requested a lite review from Copilot August 6, 2026 12:06

Copilot started reviewing on behalf of lfmeadow August 6, 2026 12:07 View session

Copilot AI reviewed Aug 6, 2026

View reviewed changes

Comment thread projects/rccl/src/device/ce_reduce/generate.py

Comment thread projects/rccl/src/device/ce_reduce/generate.py Outdated

lfmeadow force-pushed the users/lmeadows/ce-reduce-split-kernels branch from 43e7a73 to c6d137f Compare August 6, 2026 12:16

lfmeadow marked this pull request as ready for review August 6, 2026 16:35

lfmeadow requested a review from a team as a code owner August 6, 2026 16:35

alex-breslow-amd reviewed Aug 6, 2026

View reviewed changes

Comment thread projects/rccl/src/device/ce_reduce/generate.py

lfmeadow force-pushed the users/lmeadows/ce-reduce-split-kernels branch from c6d137f to 6a71529 Compare August 6, 2026 18:46

lfmeadow force-pushed the users/lmeadows/ce-reduce-split-kernels branch from fcde209 to a04c4fd Compare August 6, 2026 19:00

alex-breslow-amd reviewed Aug 6, 2026

View reviewed changes

Comment thread projects/rccl/test/CMakeLists.txt

alex-breslow-amd reviewed Aug 6, 2026

View reviewed changes

Comment thread projects/rccl/src/device/ce_reduce/ce_reduce_impl.h.in

mustafabar approved these changes Aug 6, 2026

View reviewed changes

Comment thread projects/rccl/src/device/ce_reduce/ce_reduce_impl.h.in

Comment thread projects/rccl/src/device/ce_reduce/ce_reduce_impl.h.in

Comment thread projects/rccl/src/device/ce_reduce/ce_reduce_impl.h.in

lfmeadow mentioned this pull request Aug 6, 2026

rccl: audit unroll(disable) coverage in ce_reduce_impl.h.in's rank/elementwise loops #9797

Open

lfmeadow force-pushed the users/lmeadows/ce-reduce-split-kernels branch from a04c4fd to fc5a4d1 Compare August 6, 2026 20:38

lfmeadow closed this Aug 6, 2026

lfmeadow reopened this Aug 6, 2026

alex-breslow-amd approved these changes Aug 6, 2026

View reviewed changes

lfmeadow force-pushed the users/lmeadows/ce-reduce-split-kernels branch 2 times, most recently from 23b946a to 8ea2e99 Compare August 7, 2026 16:22

lfmeadow and others added 5 commits August 7, 2026 14:14

lfmeadow force-pushed the users/lmeadows/ce-reduce-split-kernels branch from 8ea2e99 to 38f648f Compare August 7, 2026 19:15

lfmeadow requested a review from a team August 7, 2026 19:15

alex-breslow-amd merged commit d7c8e94 into develop Aug 8, 2026
33 of 38 checks passed

alex-breslow-amd deleted the users/lmeadows/ce-reduce-split-kernels branch August 8, 2026 06:19

alex-breslow-amd mentioned this pull request Aug 8, 2026

test(rccl): gate build time against the PR's base commit #9890

Open

8 tasks

Navigation Menu

Uh oh!

perf(rccl): split ce_reduce into per-kernel TUs to fix ~48min build time - #9756

perf(rccl): split ce_reduce into per-kernel TUs to fix ~48min build time#9756
alex-breslow-amd merged 5 commits into
developfrom
users/lmeadows/ce-reduce-split-kernels

lfmeadow commented Aug 6, 2026 •

edited

Loading

Uh oh!

therock-pr-bot Bot commented Aug 6, 2026 •

edited

Loading

Uh oh!

therock-pr-bot Bot commented Aug 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alex-breslow-amd commented Aug 6, 2026

Uh oh!

lfmeadow commented Aug 6, 2026

Uh oh!

Uh oh!

Uh oh!

mustafabar left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

alex-breslow-amd left a comment •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

lfmeadow commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

therock-pr-bot Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All Policy Checks Passed

Uh oh!

therock-pr-bot Bot commented Aug 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alex-breslow-amd commented Aug 6, 2026

Uh oh!

lfmeadow commented Aug 6, 2026

Uh oh!

Uh oh!

Uh oh!

mustafabar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 6, 2026

RCCL Perf-Regression Gate: ✅ PASS

Uh oh!

alex-breslow-amd left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lfmeadow commented Aug 6, 2026 •

edited

Loading

therock-pr-bot Bot commented Aug 6, 2026 •

edited

Loading

alex-breslow-amd left a comment •

edited

Loading