Skip to content

Commit 8b495ad

Browse files
committed
[CUDA][HIP] Fix build info log reporting
We were always returning the maximum log size (of 8192) even when the actual log was much shorter. Strictly speaking, we should be returning the log only up to the first null terminator. Doing otherwise could cause strange behaviour, and may account for the flaky fails we've seen in this test on both adapters.
1 parent a1094cd commit 8b495ad

File tree

5 files changed

+28
-11
lines changed

5 files changed

+28
-11
lines changed

source/adapters/cuda/program.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -345,16 +345,24 @@ urProgramGetBuildInfo(ur_program_handle_t hProgram, ur_device_handle_t hDevice,
345345
UrReturnHelper ReturnValue(propSize, pPropValue, pPropSizeRet);
346346

347347
switch (propName) {
348-
case UR_PROGRAM_BUILD_INFO_STATUS: {
348+
case UR_PROGRAM_BUILD_INFO_STATUS:
349349
return ReturnValue(hProgram->BuildStatus);
350-
}
351350
case UR_PROGRAM_BUILD_INFO_OPTIONS:
352351
return ReturnValue(hProgram->BuildOptions.c_str());
353-
case UR_PROGRAM_BUILD_INFO_LOG:
354-
return ReturnValue(hProgram->InfoLog, hProgram->MaxLogSize);
355-
case UR_PROGRAM_BUILD_INFO_BINARY_TYPE: {
356-
return ReturnValue(hProgram->BinaryType);
352+
case UR_PROGRAM_BUILD_INFO_LOG: {
353+
// We only know the maximum log length, which CUDA guarantees will include
354+
// the null terminator.
355+
// To determine the actual length of the log, search for the first
356+
// null terminator, not searching past the known maximum. If that does find
357+
// one, it will return the length excluding the null terminator, so remember
358+
// to include that.
359+
auto LogLen =
360+
std::min(hProgram->MaxLogSize,
361+
strnlen(hProgram->InfoLog, hProgram->MaxLogSize) + 1);
362+
return ReturnValue(hProgram->InfoLog, LogLen);
357363
}
364+
case UR_PROGRAM_BUILD_INFO_BINARY_TYPE:
365+
return ReturnValue(hProgram->BinaryType);
358366
default:
359367
break;
360368
}

source/adapters/hip/program.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,18 @@ urProgramGetBuildInfo(ur_program_handle_t hProgram, ur_device_handle_t,
364364
return ReturnValue(hProgram->BuildStatus);
365365
case UR_PROGRAM_BUILD_INFO_OPTIONS:
366366
return ReturnValue(hProgram->BuildOptions.c_str());
367-
case UR_PROGRAM_BUILD_INFO_LOG:
368-
return ReturnValue(hProgram->InfoLog, hProgram->MAX_LOG_SIZE);
367+
case UR_PROGRAM_BUILD_INFO_LOG: {
368+
// We only know the maximum log length, which (we assume) HIP guarantees
369+
// will include the null terminator, like CUDA does.
370+
// To determine the actual length of the log, search for the first null
371+
// terminator, not searching past the known maximum. If that does find one,
372+
// it will return the length excluding the null terminator, so remember to
373+
// include that.
374+
auto LogLen =
375+
std::min(hProgram->MAX_LOG_SIZE,
376+
strnlen(hProgram->InfoLog, hProgram->MAX_LOG_SIZE) + 1);
377+
return ReturnValue(hProgram->InfoLog, LogLen);
378+
}
369379
case UR_PROGRAM_BUILD_INFO_BINARY_TYPE:
370380
return ReturnValue(hProgram->BinaryType);
371381
default:

test/conformance/program/program_adapter_cuda.match

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ urProgramBuildTest.BuildFailure/NVIDIA_CUDA_BACKEND___{{.*}}_
22
{{OPT}}urProgramCreateWithILTest.Success/NVIDIA_CUDA_BACKEND___{{.*}}
33
{{OPT}}urProgramCreateWithILTest.SuccessWithProperties/NVIDIA_CUDA_BACKEND___{{.*}}
44
{{OPT}}urProgramCreateWithILTest.BuildInvalidProgram/NVIDIA_CUDA_BACKEND___{{.*}}
5-
{{OPT}}urProgramGetBuildInfoSingleTest.LogIsNullTerminated/NVIDIA_CUDA_BACKEND___{{.*}}
65
{{OPT}}urProgramGetInfoTest.Success/NVIDIA_CUDA_BACKEND___{{.*}}
76
{{OPT}}urProgramGetInfoTest.Success/NVIDIA_CUDA_BACKEND___{{.*}}
87
{{OPT}}urProgramGetInfoTest.Success/NVIDIA_CUDA_BACKEND___{{.*}}

test/conformance/program/program_adapter_hip.match

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
urProgramBuildTest.BuildFailure/AMD_HIP_BACKEND___{{.*}}_
22
# HIP hasn't implemented urProgramCreateWithNativeHandleTest
33
{{OPT}}urProgramCreateWithNativeHandleTest.Success/AMD_HIP_BACKEND___{{.*}}_
4-
# This test flakily fails
5-
{{OPT}}urProgramGetBuildInfoSingleTest.LogIsNullTerminated/AMD_HIP_BACKEND___{{.*}}_
64
# HIP doesn't expose kernel numbers or names
75
urProgramGetInfoTest.Success/AMD_HIP_BACKEND___{{.*}}___UR_PROGRAM_INFO_NUM_KERNELS
86
urProgramGetInfoTest.Success/AMD_HIP_BACKEND___{{.*}}___UR_PROGRAM_INFO_KERNEL_NAMES

test/conformance/program/urProgramGetBuildInfo.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ TEST_P(urProgramGetBuildInfoSingleTest, LogIsNullTerminated) {
7575

7676
ASSERT_SUCCESS(urProgramGetBuildInfo(
7777
program, device, UR_PROGRAM_BUILD_INFO_LOG, 0, nullptr, &logSize));
78+
// The size should always include the null terminator.
79+
ASSERT_GT(logSize, 0);
7880
log.resize(logSize);
7981
log[logSize - 1] = 'x';
8082
ASSERT_SUCCESS(urProgramGetBuildInfo(program, device,

0 commit comments

Comments
 (0)