-
Notifications
You must be signed in to change notification settings - Fork 745
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
[SYCL] Support memory clock rate and memory bus width queries #7412
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spec change LGTM
@@ -3180,7 +3180,40 @@ pi_result piDeviceGetInfo(pi_device Device, pi_device_info ParamName, | |||
} | |||
return ReturnValue(FreeMemory); | |||
} | |||
case PI_EXT_INTEL_DEVICE_INFO_MEMORY_CLOCK_RATE: { | |||
// If there are no any memory modules then return 0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's document this behavior in pi.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And in the .md too, and also explain what is returned when a device has multiple memories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated pi.h and document.
|
||
# Memory Clock Rate # | ||
|
||
A new device descriptor will be added which will provide the maximum clock rate of device's global memory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new device descriptor will be added which will provide the maximum clock rate of device's global memory. | |
A new device descriptor is added which provides the maximum clock rate of device's global memory. |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just a few documentation asks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
E2E test: intel/llvm-test-suite#1386