Skip to content

Commit

Permalink
Minimal changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@jmachado-amd
jmachado-amd committed Jun 18, 2024
1 parent d6312df commit cd24293
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions clients/common/containers/d_vector.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,19 @@ class d_vector

T* device_vector_setup()
{
T* d = nullptr;
T* d;
if((hipMalloc)(&d, bytes) != hipSuccess)
{
fmt::print(stderr, "Error allocating {} bytes ({} GB)\n", bytes, bytes >> 30);

This comment has been minimized.

Sign in to view

Copy link
@EdDAzevedo

EdDAzevedo Jun 19, 2024

Contributor

Minor comment: Perhaps you would like to be consistent to capture the status from hipMalloc and generate a similar error message that contain hipGetErrorString(status)?

Personally, I prefer your original version in declaring "T * d = nullptr" to always set a known value to temporary variables, and to check value of "if (d != nullptr)" instead of using the "else". I find the logic easier to understand.

Just a minor suggestion.

This comment has been minimized.

Sign in to view

Copy link
@jmachado-amd

jmachado-amd Jun 27, 2024

Author Collaborator

I personally think the new logic was slightly better, but both work and I have no strong opinions on this one, so I am reverting to how it was before the previous changes.

Regarding the error message, the new piece of code reproduces the macro CHECK_HIP_ERROR (which is used elsewhere in this file but can't be used for the hipMemset call). Since the code that calls hipMalloc is not meant to be part of this PR, it is better to just leave the error message as it is and consider improving it when these containers are refactored.
d = nullptr;
}
/* CHECK_HIP_ERROR((hipMemset)(d, 0, bytes)); // Why this doesn't work? */
if(d != nullptr)
else
{
auto error = (hipMemset)(d, 0, bytes);
if(error != hipSuccess)
auto status = (hipMemset)(d, 0, bytes);
/* CHECK_HIP_ERROR(status); // Why this doesn't work? */
if(status != hipSuccess)
{
fmt::print(stderr, "error: {} ({}) at {}:{}\n", hipGetErrorString(error), error,
fmt::print(stderr, "error: {} ({}) at {}:{}\n", hipGetErrorString(status), status,
__FILE__, __LINE__);
rocblas_abort();
}
Expand Down

0 comments on commit cd24293

Please sign in to comment.