[SYCL] [FPGA] Add latency control feature to FPGA extension docs #4917
Conversation
gmlueck
left a comment
gmlueck
left a comment
There was a problem hiding this comment.
In addition to the comments below, I also have a global comment. These specs both have a feature-test macro defined, and you are adding a new API to an existing extension. Therefore, you should bump the version of the feature-test macro to 2 and clearly describe which APIs are available in version 1 of the API vs. version 2. For example, you might include a comment in the code synopsis stating which APIs are available only in version 2:
// Added in version 2 of this extension.
template <typename Properties>
static dataT read( Properties p ); // p must be an ext::oneapi::properties
|
I forked the specs with #4961. Once merged, these changes should be rebased on the newly named copies, to prepare for an upcoming extension repo reorganization |
|
|
||
| Read/write methods take a property_list argument, which can contain the latency control properties `latency_anchor_id` and/or `latency_constraint`. | ||
|
|
||
| * `sycl::ext::intel::latency_anchor_id<N>`, where `N` is an integer: An ID to associate with the current read/write function call, which can then be referenced by other `latency_constraint` properties elsewhere in the program to define relative latency constaints. ID must be unique within the application, and a diagnostic is required if that condition is not met. |
There was a problem hiding this comment.
Having N as an integer looks very low-level.
I have the feeling it represents more a name like a pipe, kernel or specialization_constant which can be lowered by the runtime to an ID.
There was a problem hiding this comment.
This N is just a symbol to demonstrate the syntax of this property. It is a compile-time constant.
| int Z = PrefetchingLSU::load(input_ptr + 2, | ||
| property_list{latency_anchor_id<1>}); | ||
| // Store occurs 5 cycles after the anchor 1 read | ||
| BurstCoalescedLSU::store(output_ptr + 2, Z, | ||
| property_list{latency_constraint<1, latency::exact, 5>}); |
There was a problem hiding this comment.
Since you have already defined a lot of type aliases, it would be nice to have some examples defining some functions so you can write
auto Z = PrefetchingAnchorLoad(input_ptr + 2);
Z.BurstCoalescedExactLatencyStore(output_ptr + 2, 1);with Z being a wrapping type of an int and carrying all the information for you.
There was a problem hiding this comment.
As for LSU types, we recommend using alias for them because their type names are heavily reused when calling their member functions, so alias would help eliminating duplications.
However, as for their member functions, since latency control properties could be different in each call, alias would not help a lot here.
|
Changes will be made to new duplicates of these docs in #4980. Close this PR. |
4 participants
Latency control parameters will be passed to FPGA pipe and FPGA LSU member functions via
Properties. Note that the name convention ofPropertiestype is not finalized yet, and usage ofPropertiesin these two extension documents can be revised once the decisions firm up.One minor change in FPGALsu.md: removing the
cl::namespace according to the SYCL 2020 spec.