diff --git a/CMakeLists.txt b/CMakeLists.txt index 892968cce3..f097753838 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -766,85 +766,6 @@ endforeach() # available before compiling the unit tests executable(s) add_custom_target(hipify_all DEPENDS ${HIP_SOURCES}) -if (BUILD_TESTS) - if (ROCM_VERSION VERSION_GREATER_EQUAL "60400" AND CMAKE_BUILD_TYPE MATCHES "Debug") - - set(HIPIFY_SRC_DIR "${PROJECT_BINARY_DIR}/hipify/src") - set(REPLACE_SCRIPT "${CMAKE_SOURCE_DIR}/tools/scripts/replace_static.sh") - message ("Replacing static functions in ${HIPIFY_SRC_DIR} with ${REPLACE_SCRIPT} for unit tests") - # Create a list of files which needs to be modified to remove static - set(TEST_NONSTATIC_SOURCE_FILES - ${HIPIFY_SRC_DIR}/misc/alt_rsmi.cc - ${HIPIFY_SRC_DIR}/register/coll_reg.cc - ${HIPIFY_SRC_DIR}/transport/shm.cc - ${HIPIFY_SRC_DIR}/transport/p2p.cc - ) - - set(EXCLUDE_STATIC_FILE "${CMAKE_SOURCE_DIR}/tools/scripts/exclude_static_list.txt") - # Read the exclude list file into a CMake variable - file(READ "${EXCLUDE_STATIC_FILE}" EXCLUDE_STATIC_CONTENTS) - string(REPLACE "\n" ";" EXCLUDE_STATIC_LINES "${EXCLUDE_STATIC_CONTENTS}") - # Create a mapping from full/relative filename to exclude list - unset(EXCLUDE_MAP) - foreach(line ${EXCLUDE_STATIC_LINES}) - if(line MATCHES "^([a-zA-Z0-9_./-]+):([a-zA-Z0-9_,]*)") - set(fname "${CMAKE_MATCH_1}") - set(exlist "${CMAKE_MATCH_2}") - # Map both the basename and the full/relative path for flexibility - get_filename_component(basename "${fname}" NAME) - set(EXCLUDE_MAP_${fname} "${exlist}") - set(EXCLUDE_MAP_${basename} "${exlist}") - endif() - endforeach() - - # Now, for each file, get the exclude list and pass to the script - # Create a custom command to backup the original files and remove static - # Always run replace script on hipified files, but preserve original backups - foreach(srcfile ${TEST_NONSTATIC_SOURCE_FILES}) - # Try to match using the full/relative path first, then fallback to basename - set(exclude_list "") - if(DEFINED EXCLUDE_MAP_${srcfile}) - set(exclude_list "${EXCLUDE_MAP_${srcfile}}") - else() - get_filename_component(basename "${srcfile}" NAME) - if(DEFINED EXCLUDE_MAP_${basename}) - set(exclude_list "${EXCLUDE_MAP_${basename}}") - endif() - endif() - add_custom_command( - OUTPUT "${srcfile}.staticbak" - COMMAND bash -c "\ - ${CMAKE_COMMAND} -E echo 'Processing ${srcfile} for static replacement' && \ - if [ ! -f '${srcfile}.staticbak' ]; then \ - ${CMAKE_COMMAND} -E copy '${srcfile}' '${srcfile}.staticbak' && \ - ${CMAKE_COMMAND} -E echo 'Created backup: ${srcfile}.staticbak'; \ - fi && \ - ${CMAKE_COMMAND} -E echo 'Exposing internal functions/variables in ${srcfile}' && \ - ${REPLACE_SCRIPT} ${srcfile} --replace-vars --exclude-list=${exclude_list} 2>&1 \ - " - DEPENDS "${srcfile}" "${REPLACE_SCRIPT}" hipify_all - COMMENT "Removing static from ${srcfile} (backup preserved at ${srcfile}.staticbak)" - VERBATIM - ) - list(APPEND STATIC_BAK_FILES "${srcfile}.staticbak") - endforeach() - add_custom_target(replace_static_in_hipify ALL DEPENDS ${STATIC_BAK_FILES}) - add_dependencies(replace_static_in_hipify hipify_all) - else() - # Restore originals in the build directory if backup exists - foreach(srcfile ${HIPIFY_SRC_FILES}) - if(EXISTS "${srcfile}.staticbak") - execute_process( - COMMAND ${CMAKE_COMMAND} -E copy "${srcfile}.staticbak" "${srcfile}" - ) - execute_process( - COMMAND ${CMAKE_COMMAND} -E remove "${srcfile}.staticbak" - ) - endif() - endforeach() - endif() -endif() - # Generate device/host tables and all the collective functions that are going to be in librccl.so #================================================================================================== find_package(Python3 COMPONENTS Interpreter REQUIRED) @@ -918,11 +839,6 @@ add_library(rccl ${HIP_SOURCES}) ## Ensure git version is updated before building rccl add_dependencies(rccl update_git_version) -if (BUILD_TESTS AND ROCM_VERSION VERSION_GREATER_EQUAL "60400" AND CMAKE_BUILD_TYPE MATCHES "Debug") - ## Set static replacement dependency for fixture unit tests - add_dependencies(rccl replace_static_in_hipify) -endif() - ## Set RCCL include directories target_include_directories(rccl PRIVATE ${PROJECT_BINARY_DIR}/include) # for generated rccl.h header target_include_directories(rccl PRIVATE ${HIPIFY_DIR}/src) # for hipfied headers diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 4418b4f648..6767fd903f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -133,8 +133,6 @@ if(BUILD_TESTS) # Create rccl-UnitTestsFixtures binary if ROCm version is 4.6.0 or greater # and build type is Debug if (ROCM_VERSION VERSION_GREATER_EQUAL "60400" AND CMAKE_BUILD_TYPE MATCHES "Debug") - add_dependencies(rccl-UnitTests replace_static_in_hipify) - # Add rccl-UnitTestsFixtures binary list(APPEND RCCL_TEST_EXECUTABLES rccl-UnitTestsFixtures) @@ -144,7 +142,6 @@ if(BUILD_TESTS) ParamTests.cpp ArgCheckTests.cpp BitOpsTests.cpp - CollRegTests.cpp CommTests.cpp EnqueueTests.cpp IpcsocketTests.cpp @@ -160,7 +157,6 @@ if(BUILD_TESTS) ) add_executable(rccl-UnitTestsFixtures ${TEST_FIXTURE_SOURCE_FILES}) - add_dependencies(rccl-UnitTestsFixtures replace_static_in_hipify) endif() foreach(test_executable IN LISTS RCCL_TEST_EXECUTABLES) diff --git a/test/CollRegTests.cpp b/test/CollRegTests.cpp deleted file mode 100644 index 0eb6c6ba0f..0000000000 --- a/test/CollRegTests.cpp +++ /dev/null @@ -1,102 +0,0 @@ -/************************************************************************* - * Copyright (c) 2023 Advanced Micro Devices, Inc. All rights reserved. - * - * See LICENSE.txt for license information - ************************************************************************/ - -#include -#include - -#include "TestBed.hpp" - -#include "CollRegUtils.hpp" - -namespace RcclUnitTesting -{ - -TEST(CollReg, RegisterCheckP2P_ConnectedWithP2PFlag) { - ncclComm comm = {}; - ncclConnector conn = {}; - conn.connected = 1; - conn.conn.flags = NCCL_P2P_WRITE; - - bool needReg = false; - EXPECT_EQ(registerCheckP2PConnection(&comm, &conn, nullptr, 1, &needReg), ncclSuccess); - EXPECT_TRUE(needReg); -} - -TEST(CollReg, RegisterCheckP2P_ConnectedWithNoP2PFlag) { - ncclComm comm = {}; - ncclConnector conn = {}; - conn.connected = 1; - conn.conn.flags = 0; - - bool needReg = true; - EXPECT_EQ(registerCheckP2PConnection(&comm, &conn, nullptr, 1, &needReg), ncclSuccess); - EXPECT_FALSE(needReg); -} - -TEST(CollReg, RegisterCheckP2P_NotConnected_CanConnectTrue) { - // Save original canConnect - ScopedCanConnectOverride _(&(*ncclTransports[0]), mockCanConnect); - auto* originalCanConnect = ncclTransports[0]->canConnect; - ncclTransports[0]->canConnect = mockCanConnect; - - ncclComm comm = {}; - comm.rank = 0; - - ncclPeerInfo peers[2] = {}; - comm.peerInfo = peers; - - // Set myInfo and peerInfo - peers[0].hostHash = 123; - peers[0].pidHash = 999; - peers[0].cudaDev = 0; - peers[0].busId = 0x1; - peers[0].comm = &comm; - - peers[1].hostHash = 123; - peers[1].pidHash = 888; - peers[1].cudaDev = 1; - peers[1].busId = 0x2; - peers[1].comm = &comm; - - ncclConnector conn = {}; - conn.connected = 0; - - ncclTopoGraph graph = {}; - - bool needReg = false; - - EXPECT_EQ(registerCheckP2PConnection(&comm, &conn, &graph, 1, &needReg), ncclSuccess); - EXPECT_TRUE(needReg); // Because mockCanConnect returns true - - // Restore original canConnect - ncclTransports[0]->canConnect = originalCanConnect; -} - -TEST(CollReg, RegisterCheckP2P_NotConnected_CanConnectFalse) { - ScopedCanConnectOverride _(&(*ncclTransports[0]), mockCanConnectFalse); - - ncclComm comm = {}; - comm.rank = 0; - ncclPeerInfo peers[2] = {}; - comm.peerInfo = peers; - - // Set up myInfo and peerInfo to block P2P - peers[0].hostHash = 123; peers[0].pidHash = 999; - //peers[0].dev = 0; - peers[1].hostHash = 456; peers[1].pidHash = 888; // different host - //peers[1].dev = 1; - - ncclConnector conn = {}; - conn.connected = 0; - - ncclTopoGraph graph = {}; - - bool needReg = true; - EXPECT_EQ(registerCheckP2PConnection(&comm, &conn, &graph, 1, &needReg), ncclSuccess); - EXPECT_FALSE(needReg); // should be false for different host -} - -} diff --git a/test/common/CollRegUtils.hpp b/test/common/CollRegUtils.hpp deleted file mode 100644 index 9e31a4c9b3..0000000000 --- a/test/common/CollRegUtils.hpp +++ /dev/null @@ -1,52 +0,0 @@ -/************************************************************************* - * Copyright (c) 2023 Advanced Micro Devices, Inc. All rights reserved. - * - * See LICENSE.txt for license information - ************************************************************************/ - -#include -#include -#include -#include -#include "TestBed.hpp" - -ncclResult_t registerCheckP2PConnection(struct ncclComm* comm, struct ncclConnector* conn, struct ncclTopoGraph* graph, int peer, bool* needReg); - -// Global mock transport definition (outside any namespace) -static ncclResult_t mockCanConnect(int* result, struct ncclComm*, struct ncclTopoGraph*, - struct ncclPeerInfo*, struct ncclPeerInfo*) { -*result = 1; -return ncclSuccess; -} - -static ncclResult_t mockCanConnectFalse(int* result, struct ncclComm*, struct ncclTopoGraph*, - struct ncclPeerInfo*, struct ncclPeerInfo*) { -*result = 0; // Simulate failure to connect -return ncclSuccess; -} - -static struct ncclTransport mockTransport = { -.canConnect = mockCanConnect, -}; - -struct ncclTransport* ncclTransports[] = { -&mockTransport, -nullptr -}; - -struct ScopedCanConnectOverride { - struct ncclTransport* transport; - ncclResult_t (*originalFn)(int*, struct ncclComm*, struct ncclTopoGraph*, - struct ncclPeerInfo*, struct ncclPeerInfo*); - - ScopedCanConnectOverride(struct ncclTransport* t, - ncclResult_t (*mockFn)(int*, struct ncclComm*, struct ncclTopoGraph*, - struct ncclPeerInfo*, struct ncclPeerInfo*)) - : transport(t), originalFn(t->canConnect) { - t->canConnect = mockFn; - } - - ~ScopedCanConnectOverride() { - transport->canConnect = originalFn; - } -}; diff --git a/tools/scripts/replace_static.sh b/tools/scripts/replace_static.sh deleted file mode 100755 index 5f45f60dcf..0000000000 --- a/tools/scripts/replace_static.sh +++ /dev/null @@ -1,128 +0,0 @@ -#!/bin/bash -# Copyright (c) 2025 Advanced Micro Devices, Inc. All rights reserved. -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the "Software"), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in -# all copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -# THE SOFTWARE. - -# Usage: -# ./replace_static.sh [--replace-vars] [--verbose] [--exclude-list=func1,func2,var1] -# -# - Replaces all 'static' function definitions with non-static. -# - Replaces all 'static inline' with 'inline'. -# - If --replace-vars is given, also replaces 'static' at variable definitions. -# - If --exclude-list is given, skips listed functions/variables. -# - If --verbose is given, shows a diff of the changes. - -set -e - -SOURCE_FILE="$1" -shift - -REPLACE_VARS=0 -VERBOSE=0 -EXCLUDE_LIST="" - -for arg in "$@"; do - case "$arg" in - --replace-vars) REPLACE_VARS=1 ;; - --verbose) VERBOSE=1 ;; - --exclude-list=*) EXCLUDE_LIST="${arg#*=}" ;; - esac -done - -if [[ ! -f "$SOURCE_FILE" ]]; then - echo "Source file '$SOURCE_FILE' not found!" - exit 1 -fi - -TMP_FILE="${SOURCE_FILE}.tmp.$$" -cp "$SOURCE_FILE" "$TMP_FILE" - -# Prepare exclude regex if needed -if [[ -n "$EXCLUDE_LIST" ]]; then - # Convert comma-separated list to alternation regex - EXCLUDE_REGEX="($(echo "$EXCLUDE_LIST" | sed 's/,/|/g'))" -fi - -# Mark lines with excluded function or variable names -if [[ -n "$EXCLUDE_LIST" ]]; then - IFS=',' read -ra EXCLUDES <<< "$EXCLUDE_LIST" - for name in "${EXCLUDES[@]}"; do - # Mark function definitions/declarations to skip (robust to qualifiers/types) - sed -E -i "/static[[:space:]]+([a-zA-Z_][a-zA-Z0-9_:[:space:]\*\&]*)[[:space:]]+${name}[[:space:]]*(\(|;)/s/^/__STATIC_SKIP__/" "$TMP_FILE" - # Mark variable definitions/declarations to skip (no '(' on the line) - sed -E -i '/\(/!s/static[[:space:]]+.*\b'"${name}"'\b[[:space:]]*(=|;)/__STATIC_SKIP__&/' "$TMP_FILE" - done -fi - -# s/\bstatic[[:space:]]+inline\b/inline/g - # - Matches 'static' followed by one or more spaces and then 'inline' as a whole word. - # - Replaces it with just 'inline'. - # - Example: 'static inline int foo()' -> 'inline int foo()' -# s/^([[:space:]]*(inline[[:space:]]+|__device__[[:space:]]+|__forceinline__[[:space:]]+|__host__[[:space:]]+|__global__[[:space:]]+|)*)static[[:space:]]+/\1/g - # - Matches lines that start with optional whitespace, then any qualifiers (inline, __device__, etc.), then 'static' and spaces. - # - Removes 'static' but preserves the qualifiers and indentation. - # - Example: ' inline static int foo()' -> ' inline int foo()' -sed -E -i '/^__STATIC_SKIP__/!{ - s/\bstatic[[:space:]]+inline\b/inline/g - s/^([[:space:]]*(inline[[:space:]]+|__device__[[:space:]]+|__forceinline__[[:space:]]+|__host__[[:space:]]+|__global__[[:space:]]+|)*)static[[:space:]]+/\1/g -}' "$TMP_FILE" - -# # Always remove 'static' from function definitions/declarations, except excluded -# sed -E -i '/^__STATIC_SKIP__/!s/^([[:space:]]*(inline[[:space:]]+|__device__[[:space:]]+|__forceinline__[[:space:]]+|__host__[[:space:]]+|__global__[[:space:]]+|)*)static[[:space:]]+/\1/g' "$TMP_FILE" - -# # Replace 'static inline' with 'inline' everywhere except marked lines -# sed -E -i '/^__STATIC_SKIP__/!s/\bstatic[[:space:]]+inline\b/inline/g' "$TMP_FILE" - -# Remove 'static' at variable definitions, excluding variables in EXCLUDE_LIST -if [[ "$REPLACE_VARS" == "1" ]]; then - if [[ -n "$EXCLUDE_LIST" ]]; then - # Regex explanation: - # '/^__STATIC_SKIP__/{b}; /\(/b; s/^([[:space:]]*)static([[:space:]]+)/\1/g' - # - /^__STATIC_SKIP__/{b}; If the line starts with __STATIC_SKIP__, branch (skip substitution). - # - /\(/b; If the line contains '(', branch (skip substitution; likely a function). - # - s/^([[:space:]]*)static([[:space:]]+)/\1/g - # - Matches 'static' at the start of a line (possibly after indentation). - # - Removes 'static', preserving indentation. - # - Only applies to lines not skipped above (i.e., not excluded and not functions). - sed -E -i '/^__STATIC_SKIP__/{b}; /\(/b; s/^([[:space:]]*)static([[:space:]]+)/\1/g' "$TMP_FILE" - else - # Regex explanation: - # '/\(/!s/^([[:space:]]*)static([[:space:]]+)/\1/g' - # - /\(/! Only apply to lines that do NOT contain '(' (i.e., not functions). - # - s/^([[:space:]]*)static([[:space:]]+)/\1/g - # - Matches 'static' at the start of a line (possibly after indentation). - # - Removes 'static', preserving indentation. - sed -E -i '/\(/!s/^([[:space:]]*)static([[:space:]]+)/\1/g' "$TMP_FILE" - fi -fi - -# Remove the marker after all substitutions, preserving original line formatting -sed -E -i 's/([[:space:]]*)__STATIC_SKIP__/\1/' "$TMP_FILE" - -if [[ "$VERBOSE" == "1" ]]; then - echo "[INFO] Showing diff for changes:" - diff -u "$SOURCE_FILE" "$TMP_FILE" || true -fi - -mv "$TMP_FILE" "$SOURCE_FILE" -echo "Static function replacement complete for $SOURCE_FILE" -if [[ "$REPLACE_VARS" == "1" ]]; then - echo "Static variable replacement also performed." -fi -