AI/rocm-systems
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove static to non-static conversion used in tests (#2084)

Browse Source 
* Remove coll_reg tests which are unsupported

* removed static to non-static conversion feature
This commit is contained in:
Atul Kulkarni
2025-12-04 18:03:14 -06:00
committed by GitHub
parent 892d258319
commit 7ec8e73e12
5 changed files with 0 additions and 370 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CMakeLists.txt
-84
View File
@@ -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
test/CMakeLists.txt
-4
View File
@@ -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)
test/CollRegTests.cpp
-102
View File
@@ -1,102 +0,0 @@
/*************************************************************************
* Copyright (c) 2023 Advanced Micro Devices, Inc. All rights reserved.
*
* See LICENSE.txt for license information
************************************************************************/
#include <gtest/gtest.h>
#include <rccl/rccl.h>
#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
}
}
test/common/CollRegUtils.hpp
-52
View File
@@ -1,52 +0,0 @@
/*************************************************************************
* Copyright (c) 2023 Advanced Micro Devices, Inc. All rights reserved.
*
* See LICENSE.txt for license information
************************************************************************/
#include <gtest/gtest.h>
#include <rccl/rccl.h>
#include <comm.h>
#include <transport.h>
#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;
}
};
tools/scripts/replace_static.sh
-128
View File
@@ -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 <source_file> [--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