[SYCL][HIP] Vendor ID for AMD devices is expected to be 4098 #6127
Conversation
sycl/plugins/hip/pi_hip.cpp
Outdated
| @@ -980,7 +980,7 @@ pi_result hip_piDeviceGetInfo(pi_device device, pi_device_info param_name, | |||
| PI_DEVICE_TYPE_GPU); | |||
| } | |||
| case PI_DEVICE_INFO_VENDOR_ID: { | |||
| return getInfo(param_value_size, param_value, param_value_size_ret, 4318u); | |||
| return getInfo(param_value_size, param_value, param_value_size_ret, 4098u); | |||
There was a problem hiding this comment.
Out of curiosity, what vendor ID is returned when DPC++ was built with HIP NVIDIA support?
There was a problem hiding this comment.
Thank you for your suggestion. I don't have a platform for the test, but updated the codes:
case PI_DEVICE_INFO_VENDOR_ID: {
#if defined(__HIP_PLATFORM_AMD__)
pi_uint32 vendor_id = 4098u;
#elif defined(__HIP_PLATFORM_NVIDIA__)
pi_uint32 vendor_id = 4318u;
#else
pi_uint32 vendor_id = 0u;
#endif
return getInfo(param_value_size, param_value, param_value_size_ret, vendor_id);
}
There was a problem hiding this comment.
@zjin-lcf - Are you planning on incorporating the changes you mention here?
There was a problem hiding this comment.
I checked the box 'Allow edits and access to secrets by maintainers', so assumed that reviewers could incorporate the changes.
There was a problem hiding this comment.
Just want to let you know that I'm not a maintainer so I can't push the changes. Can you please push them for me?
There was a problem hiding this comment.
Okay. I updated the codes for pull request.
pvchupin
changed the title
The indentation needs to be clang-formatted Co-authored-by: Nicolas Miller <nicolas.pierre.miller@gmail.com>
|
@npmiller |
You should use And it can also be integrated with IDEs or editors (for example I personally use this plugin to call it directly from vim: https://github.com/frasercrmck/formative.vim ) |
You can also use https://github.com/intel/llvm/blob/sycl/clang/tools/clang-format/clang-format-diff.py to apply clang-format from a diff. See the comment at the top of the file. The PR testing should also tell us when things are not formatted in accordance with recent clang-format, though it seems testing did not run for this PR until now. It should also be possible to create a git hook for it. |
|
@zjin-lcf, is there anything else that needs to be done to merge this PR? |
|
Opening the check report shows many lld errors: |
|
Failing tests were disabled in intel/llvm-test-suite#1034. I believe this is ready to be merged. |
A pull request to solve the issue #6123
Thanks for your review.