Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tests] Fix for DEV builds. Fix testing on 16GB GPUs. #1729

Merged
merged 11 commits into from
Sep 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 37 additions & 11 deletions src/hip/handlehip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,21 +81,47 @@ std::size_t GetAvailableMemory()

void* default_allocator(void*, size_t sz)
{
if(sz > GetAvailableMemory())
MIOPEN_THROW("Memory not available to allocate buffer: " + std::to_string(sz));
void* result;
auto status = hipMalloc(&result, sz);
if(status != hipSuccess)
const auto available = GetAvailableMemory();
MIOPEN_LOG_I2("GetAvailableMemory " << available);
if(sz > available)
MIOPEN_LOG_I("GetAvailableMemory reports unsufficient memory to allocate " << sz);
void* ptr;
const auto status = hipMalloc(&ptr, sz);
if(status == hipSuccess)
{
status = hipHostMalloc(&result, sz);
if(status != hipSuccess)
MIOPEN_THROW_HIP_STATUS(status,
"Hip error creating buffer " + std::to_string(sz) + ": ");
Comment on lines -92 to -93
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@muralinr I do not think we want to append + ": " to each MIOPEN_THROW_HIP_STATUS.

MIOPEN_LOG_I2("hipMalloc " << sz << " at " << ptr << " Ok");
return ptr;
}
return result;
const auto status_host = hipHostMalloc(&ptr, sz);
if(status_host == hipSuccess)
{
MIOPEN_LOG_I2("hipHostMalloc " << sz << " at " << ptr << " Ok");
return ptr;
}
MIOPEN_LOG_W("hipMalloc " << sz << " status: " << status);
MIOPEN_THROW_HIP_STATUS(status_host, "hipHostMalloc " + std::to_string(sz));
}

void default_deallocator(void*, void* mem) { hipFree(mem); }
inline std::string to_string(void* const ptr)
{
std::ostringstream oss;
oss << ptr;
return oss.str();
}

void default_deallocator(void*, void* mem)
{
size_t size = 0;
auto status = hipMemPtrGetInfo(mem, &size);
if(status != hipSuccess)
MIOPEN_LOG_W("hipMemPtrGetInfo at " << mem << " status: " << status);
status = hipFree(mem);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it ok to use hipFree for device and for host memory?

if(status != hipSuccess)
MIOPEN_THROW_HIP_STATUS(status,
"hipFree " + std::to_string(size) + " at " + to_string(mem));
else
MIOPEN_LOG_I2("hipFree " << size << " at " << mem << " Ok");
}

int get_device_id() // Get random device
{
Expand Down
2 changes: 1 addition & 1 deletion src/hip/hiperrors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace miopen {

std::string HIPErrorMessage(int error, const std::string& msg)
{
return msg + " " + hipGetErrorString(static_cast<hipError_t>(error));
return msg + ": " + hipGetErrorString(static_cast<hipError_t>(error));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is ":" change mandatory?

Copy link
Contributor Author

@atamazov atamazov Sep 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if we want to automatically separate error message from the error code, see https://github.com/ROCmSoftwarePlatform/MIOpen/pull/1729/files#r962057473.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok.

}

} // namespace miopen
4 changes: 3 additions & 1 deletion test/handle_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ void test_warnings(kernel_type_t kern_type)
if(kern_type == miopenOpenCLKernelType)
EXPECT(throws([&] {
h.AddKernel("GEMM", "", WriteNop(kern_type), "write", {1, 1, 1}, {1, 1, 1}, "");
MIOPEN_LOG_E("FAILED: Build of the OpenCL kernel should produce warnings");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is very good.

}));
else if(kern_type == miopenHIPKernelType)
EXPECT(throws([&] {
Expand All @@ -223,7 +224,8 @@ void test_warnings(kernel_type_t kern_type)
"",
0,
false,
WriteNop(miopenHIPKernelType));
WriteNop(kern_type));
MIOPEN_LOG_E("FAILED: Build of the HIP kernel 'nop_hip.cpp' should produce warnings");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. This change is good.

}));
#else
(void)kern_type;
Expand Down