Skip to content
Merged
2 changes: 1 addition & 1 deletion projects/rccl/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ include(cmake/NcclCompat.cmake)
rccl_define_nccl_compat_options()

option(BUILD_ADDRESS_SANITIZER "Enable address sanitizer" ${RCCL_DEFAULT_BUILD_ADDRESS_SANITIZER})
option(BUILD_ALL_UNROLLS "Generate every unroll factor (1,2,4,8,16,32) for the targeted GPU arch(es)" OFF)
option(BUILD_ALL_UNROLLS "Also generate unroll factors 8 and 16 on top of the per-arch default set" OFF)
option(BUILD_BFD "Enable custom backtrace (if bfd.h exists)" OFF)
option(BUILD_LOCAL_GPU_TARGET_ONLY "Build only for GPUs detected on this machine" OFF)
option(BUILD_SHARED_LIBS "Build as shared library" ON)
Expand Down
4 changes: 2 additions & 2 deletions projects/rccl/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function display_help()
echo "RCCL build & installation helper script"
echo " Options:"
echo " --address-sanitizer Build with address sanitizer enabled"
echo " --all_unrolls Build every unroll factor (1,2,4,8,16,32) for the targeted GPU arch(es) instead of the per-arch default set"
echo " --all_unrolls Also build unroll factors 8 and 16 on top of the per-arch default set"
echo " --amdgpu_targets Only compile for specified GPU architecture(s). For multiple targets, separate by ';' (builds for all supported GPU architectures by default)"
echo " --cmake-options Pass additional CMake options (e.g. --cmake-options \"-DFOO=BAR -DBAZ=ON\")"
echo " --debug Build debug library"
Expand Down Expand Up @@ -399,7 +399,7 @@ if [[ "${build_local_gpu_only}" == true ]]; then
cmake_common_options="${cmake_common_options} -DBUILD_LOCAL_GPU_TARGET_ONLY=ON"
fi

# Build every unroll factor for the targeted arch(es)
# Also build unroll 8 and 16 on top of the per-arch default
if [[ "${build_all_unrolls}" == true ]]; then
cmake_common_options="${cmake_common_options} -DBUILD_ALL_UNROLLS=ON"
fi
Expand Down
18 changes: 11 additions & 7 deletions projects/rccl/src/device/generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
# Unroll factors whose device functions are compiled for one arch only, and the single
# source of truth for that restriction: get_arch_guard(), the specialized_files.txt guard
# the device linker filters on, the local-arch unroll set, and the ncclDevFuncUnrollArch[]
# table the host checks before selecting an unroll. --all_unrolls clears it.
# table the host checks before selecting an unroll. --all_unrolls does not clear it.
Comment thread
PedramAlizadeh marked this conversation as resolved.
Resolved
unroll_arch_requirement = {
"32": "gfx1250",
}
Expand Down Expand Up @@ -132,7 +132,8 @@ def paste(sep, *args):
is_ifc = 1 if sys.argv[2] == "ON" else 0
is_local_arch_only = 1 if sys.argv[4] == "ON" else 0
is_rocshmem = 1 if sys.argv[5] == "ON" else 0
# BUILD_ALL_UNROLLS / install.sh --all_unrolls: generate every unroll for the targeted archs.
# BUILD_ALL_UNROLLS / install.sh --all_unrolls: also generate unroll 8 and 16.
# Off by default, so a normal build does not compile those factors.
build_all_unrolls = 1 if sys.argv[6] == "ON" else 0

func_pattern = sys.argv[7:8]
Expand Down Expand Up @@ -306,12 +307,15 @@ def calc_unroll_and_pipeline_for_local_arch():
# except for gfx950. For gfx950, we also disable pipelining.
local_unroll, local_pipeline = calc_unroll_and_pipeline_for_local_arch()

# --all_unrolls widens the unroll set and compiles every one for the targeted archs, so the
# pin no longer holds. The arch's pipeline decision stands. Must follow the call above, the
# only reader of unrolls_requiring_arch.
# --all_unrolls appends unroll 8 and 16 to the per-arch default. The default set and
# any arch pin stay as calc_unroll_and_pipeline_for_local_arch decided them, and the
# arch's pipeline decision stands. A build without the flag does not compile 8 or 16.
# Must follow the call above, the only reader of unrolls_requiring_arch.
if build_all_unrolls:
local_unroll = all_unrolls
unroll_arch_requirement.clear()
local_unroll = list(local_unroll)
for unroll in ("8", "16"):
Comment thread
PedramAlizadeh marked this conversation as resolved.
Resolved
if unroll not in local_unroll:
local_unroll.append(unroll)

# rocSHMEM/GDA-based collectives: only generated when ENABLE_ROCSHMEM build is requested
gda_colls = {"AlltoAllGda", "AlltoAllvGda"}
Expand Down
44 changes: 36 additions & 8 deletions projects/rccl/src/device/test_generate_device_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,26 +245,54 @@ def test_device_tables_are_index_aligned_across_unrolls(self):
% (unroll, base_unroll),
)

def test_all_unrolls_opt_in_generates_every_unroll(self):
# BUILD_ALL_UNROLLS fills in the skipped unrolls and drops the gfx1250 restriction.
def test_default_build_omits_unroll_8_and_16(self):
# Multi-arch default is 1/2/4 plus gfx1250-pinned 32. Unroll 8 and 16 stay
# out, so a default build does not compile them.
self.assertEqual({"1", "2", "4", "32"}, set(_unroll_tables(self.header)))
generated = self._unroll_table("ncclDevFuncUnrollGenerated", r"true|false")
self.assertEqual(
{
"1": "true",
"2": "true",
"4": "true",
"8": "false",
"16": "false",
"32": "true",
},
generated,
)
pinned = self._unroll_table("ncclDevFuncUnrollArch", r'nullptr|"gfx\w+"')
self.assertEqual('"gfx1250"', pinned["32"])
self.assertEqual("nullptr", pinned["8"])
self.assertEqual("nullptr", pinned["16"])
Comment thread
PedramAlizadeh marked this conversation as resolved.
Resolved

def test_all_unrolls_adds_8_and_16_without_unpinning(self):
Comment thread
PedramAlizadeh marked this conversation as resolved.
Resolved
# BUILD_ALL_UNROLLS appends 8 and 16 to the per-arch default. Unroll 32
# stays compiled for gfx1250 only, and the other defaults stay generated.
with tempfile.TemporaryDirectory(prefix="rccl_devtable_all_") as tmpdir:
header = _generate(tmpdir, all_unrolls="ON")
host = _read_generated(tmpdir, "host_table.cpp")
self.assertEqual({"1", "2", "4", "8", "16", "32"}, set(_unroll_tables(header)))
# Both host tables flip under the flag, and no other test reads them on this path.
generated = self._unroll_table("ncclDevFuncUnrollGenerated", r"true|false", host)
self.assertEqual(
["true"] * 6,
list(generated.values()),
"BUILD_ALL_UNROLLS must mark every unroll generated",
"BUILD_ALL_UNROLLS must mark the default unrolls plus 8 and 16 generated",
)
pinned = self._unroll_table("ncclDevFuncUnrollArch", r'nullptr|"gfx\w+"', host)
self.assertEqual(
["nullptr"] * 6,
list(pinned.values()),
"BUILD_ALL_UNROLLS compiles every unroll for the target, so none stays pinned",
{
"1": "nullptr",
"2": "nullptr",
"4": "nullptr",
"8": "nullptr",
"16": "nullptr",
"32": '"gfx1250"',
},
pinned,
"BUILD_ALL_UNROLLS must leave the gfx1250 pin on unroll 32",
)
self.assertNotIn(
self.assertIn(
"#if (defined(__gfx1250__) || defined(__gfx1250_strict__))\n", header
)

Expand Down
6 changes: 3 additions & 3 deletions projects/rccl/test/RcclWrapTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2900,9 +2900,9 @@ TEST(SkipPresetTopoMatching, Gfx1250_SkipsRomeModelMatching)
// test_runner/configs), and Rcclwrap.* is the only listed pattern this file
// matches. A suite of their own would never be run.
// ---------------------------------------------------------------------------
// Which unroll factors are arch-pinned is a build property, not a constant:
// BUILD_ALL_UNROLLS compiles every one for the targeted archs and pins none. Ask
// the table for a pinned factor rather than hardcoding 32.
// Which unroll factors are arch-pinned is a build property, not a constant.
// BUILD_ALL_UNROLLS adds unroll 8 and 16 and leaves an existing pin in place.
// Ask the table for a pinned factor rather than hardcoding 32.
constexpr char kOtherArch[] = "gfx1200";

TEST(Rcclwrap, UnrollFactor_RejectsArchRestrictedUnrollOnOtherArch)
Comment thread
PedramAlizadeh marked this conversation as resolved.
Resolved
Expand Down
10 changes: 6 additions & 4 deletions projects/rccl/test/kernel-count/tests/test_kernel_counts.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,12 +345,14 @@ def test_all_unrolls_opt_in_adds_the_skipped_unrolls(tmp_path_factory, generated
)

counts = _count_by(records, "unroll")
assert set(counts) == {"1", "2", "4", "8", "16", "32"}, (
"--all_unrolls did not generate every unroll: %s" % sorted(counts)
baseline = _count_by(generated["OFF"]["records"], "unroll")
assert set(counts) - set(baseline) == {"8", "16"}, (
Comment thread
PedramAlizadeh marked this conversation as resolved.
Resolved
"--all_unrolls should add only unroll 8 and 16: %s vs %s" % (sorted(counts), sorted(baseline))
)
assert set(baseline) <= set(counts), (
"--all_unrolls dropped a default unroll: %s vs %s" % (sorted(counts), sorted(baseline))
)
assert len(set(counts.values())) == 1, "--all_unrolls broke unroll lockstep: %s" % counts

baseline = _count_by(generated["OFF"]["records"], "unroll")
assert set(counts.values()) == set(baseline.values()), (
"--all_unrolls changed the per-unroll kernel count: %s vs %s" % (counts, baseline)
)
Expand Down
Loading