Skip to content

Commit 37d524b

Browse files
[RCCL] Clear HIP error state in CUCHECK and CUCHECKGOTO
macros (#4617) [rocm-systems] ROCm/rocm-systems#4617 (commit 7456cf4)
1 parent 465d184 commit 37d524b

2 files changed

Lines changed: 55 additions & 0 deletions

File tree

src/include/rocmwrap.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ typedef hsa_status_t (*PFN_hsa_amd_portable_export_dmabuf)(const void* ptr, size
3737
hipError_t err = cmd; \
3838
if( err != hipSuccess ) { \
3939
WARN("HIP failure '%s' at %s:%d", hipGetErrorString(err), __FILE__, __LINE__); \
40+
(void)hipGetLastError(); /* clear sticky HIP error state */ \
4041
return ncclUnhandledCudaError; \
4142
} \
4243
} while(false)
@@ -45,6 +46,7 @@ typedef hsa_status_t (*PFN_hsa_amd_portable_export_dmabuf)(const void* ptr, size
4546
hipError_t err = cmd; \
4647
if( err != hipSuccess ) { \
4748
WARN("HIP failure '%s' at %s:%d", hipGetErrorString(err), __FILE__, __LINE__); \
49+
(void)hipGetLastError(); /* clear sticky HIP error state */ \
4850
res = ncclUnhandledCudaError; \
4951
goto label; \
5052
} \

test/RcclWrapTests.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "common/ProcessIsolatedTestRunner.hpp"
1515
#include "debug.h"
1616
#include "graph/topo.h"
17+
#include "rocmwrap.h"
1718

1819
namespace RcclUnitTesting
1920
{
@@ -157,6 +158,14 @@ static bool isProtoStrValid(const char* envStr)
157158
return false; // No match found
158159
}
159160

161+
// Helper that exercises CUCHECK with a guaranteed-to-fail HIP call
162+
static ncclResult_t triggerCucheckFailure()
163+
{
164+
CUCHECK(hipPointerGetAttribute(nullptr, HIP_POINTER_ATTRIBUTE_CONTEXT,
165+
(hipDeviceptr_t)0x1));
166+
return ncclSuccess;
167+
}
168+
160169
// Helper function to validate algorithm string against known valid algorithms
161170
static bool isAlgoStrValid(const char* envStr)
162171
{
@@ -1306,4 +1315,48 @@ TEST(Rcclwrap, AllPxnTests)
13061315
EXPECT_TRUE(allTestsPassed) << "One or more PXN process-isolated tests failed";
13071316
}
13081317

1318+
TEST(Rcclwrap, CucheckMacro_CheckStickyHipErrorOnFailure)
1319+
{
1320+
hipError_t hipErr = hipSetDevice(0);
1321+
if(hipErr != hipSuccess)
1322+
{
1323+
GTEST_SKIP() << "No GPU available";
1324+
}
1325+
1326+
// Clear any pre-existing sticky error so we start clean
1327+
(void)hipGetLastError();
1328+
1329+
// Force a HIP failure through CUCHECK using an invalid device pointer (0x1)
1330+
ncclResult_t ret = triggerCucheckFailure();
1331+
1332+
EXPECT_EQ(ncclUnhandledCudaError, ret)
1333+
<< "CUCHECK should return ncclUnhandledCudaError on failure";
1334+
EXPECT_EQ(hipSuccess, hipGetLastError())
1335+
<< "CUCHECK must clear sticky HIP error after failure";
1336+
}
1337+
1338+
TEST(Rcclwrap, CucheckgotoMacro_CheckStickyHipErrorOnFailure)
1339+
{
1340+
hipError_t hipErr = hipSetDevice(0);
1341+
if(hipErr != hipSuccess)
1342+
{
1343+
GTEST_SKIP() << "No GPU available";
1344+
}
1345+
1346+
// Clear any pre-existing sticky error so we start clean
1347+
(void)hipGetLastError();
1348+
1349+
// Force a HIP failure through CUCHECKGOTO using an invalid device pointer (0x1)
1350+
ncclResult_t ret = ncclSuccess;
1351+
CUCHECKGOTO(hipPointerGetAttribute(nullptr, HIP_POINTER_ATTRIBUTE_CONTEXT,
1352+
(hipDeviceptr_t)0x1),
1353+
ret, check_sticky);
1354+
1355+
check_sticky:
1356+
EXPECT_EQ(ncclUnhandledCudaError, ret)
1357+
<< "CUCHECKGOTO should set result to ncclUnhandledCudaError on failure";
1358+
EXPECT_EQ(hipSuccess, hipGetLastError())
1359+
<< "CUCHECKGOTO must clear sticky HIP error after failure";
1360+
}
1361+
13091362
} // namespace RcclUnitTesting

0 commit comments

Comments
 (0)