[WIP][SYCL][FPGA] Runtime implementation for host_pipes #5851
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To align with spec: intel#5838 Host pipe is a FIFO construct that provide links between elements of design that are accessed through read/write API. Host pipe is a pipe that links device kernel with host program.
keryell
reviewed
Mar 22, 2022
| bool IsReadOp; | ||
|
|
||
| public: | ||
| CGReadWriteHostPipe(std::string Name, bool Block, void *Ptr, size_t Size, |
There was a problem hiding this comment.
I guess a lot of parameters could be const&.
| std::vector<Requirement *> Requirements, | ||
| std::vector<detail::EventImplPtr> Events, | ||
| detail::code_location loc = {}) | ||
| : CG(ReadWriteHostPipe, std::move(ArgsStorage), std::move(AccStorage), |
There was a problem hiding this comment.
I am unsure this is the best way to accept movable stuff.
| @@ -2663,6 +2672,16 @@ class __SYCL_EXPORT handler { | |||
| /// The list of valid SYCL events that need to complete | |||
| /// before barrier command can be executed | |||
| std::vector<detail::EventImplPtr> MEventsWaitWithBarrier; | |||
| /// Host pipe name | |||
| std::string HostPipeName; | |||
| /// Host pipe host pointer | |||
There was a problem hiding this comment.
Great to have comments!
But they could be more precise than just reading the variable name. I have no idea about what is the host pointer.
| private: | ||
| static constexpr int32_t m_Size = sizeof(_dataT); | ||
| static constexpr int32_t m_Alignment = alignof(_dataT); | ||
| static constexpr int32_t ID = _name::id; |
There was a problem hiding this comment.
What if the programmer use a
#define ID 42?
| @@ -7577,6 +7577,62 @@ pi_result piextUSMGetMemAllocInfo(pi_context Context, const void *Ptr, | |||
| return PI_SUCCESS; | |||
| } | |||
|
|
|||
| /// Host Pips | |||
| /// @param blocking indicate if the read and write operations are blocking or | ||
| /// non-blocking | ||
| /// @param ptr a pointer to buffer in host memory where data is to be read into | ||
| /// or written from. |
sherry-yuan
changed the title
sherry-yuan
commented
Mar 22, 2022
| #endif | ||
| host_pipe { // TODO change name to pipe, and merge into the existing pipe | ||
| // implementation | ||
| static_assert( |
There was a problem hiding this comment.
make a type trait instead
sherry-yuan
commented
Mar 22, 2022
| // TODO: Get pipe name from template, get host pointer by quering the host | ||
| // pipe registration / host pipe mapping | ||
| _dataT data; | ||
| const std::string pipe_name = std::to_string(ID); |
There was a problem hiding this comment.
Change to _get_built_in_pipe_id
sherry-yuan
commented
Mar 22, 2022
Comment on lines
+2129
to
+2132
| RT::PiProgram Program = | ||
| sycl::detail::ProgramManager::getInstance().getBuiltPIProgram( | ||
| M, Queue->getContextImplPtr(), Queue->getDeviceImplPtr(), KernelName); | ||
|
|
There was a problem hiding this comment.
Only need get the program base on the pipe type, require frontend to provide the info. No need for kernel name
sherry-yuan
commented
Mar 22, 2022
|
|
||
| return enqueueReadWriteHostPipe(MQueue, "ReadWriteHostPipeKernelName", | ||
| pipeName, blocking, hostPtr, typeSize, | ||
| RawEvents, Event, read); |
There was a problem hiding this comment.
Does it need to depend on the previous events in this command group? Do we care about data dependency?
sherry-yuan
added a commit
to sherry-yuan/llvm
that referenced
this pull request
Mar 25, 2022
Setup lower runtime extension functions for host pipes. See also intel#5766 intel#5851 Host pipe sycl spec: intel#5838
|
closing because superseded by #5894 |
sherry-yuan
added a commit
to sherry-yuan/llvm
that referenced
this pull request
Apr 21, 2022
Setup lower runtime extension functions for host pipes. See also intel#5766 intel#5851 Host pipe sycl spec: intel#5838
rho180
pushed a commit
to zibaiwan/llvm
that referenced
this pull request
Feb 24, 2023
Setup lower runtime extension functions for host pipes. See also intel#5766 intel#5851 Host pipe sycl spec: intel#5838 (cherry picked from commit e4d513c) Zibai fixed the conflict.
zibaiwan
pushed a commit
to zibaiwan/llvm
that referenced
this pull request
Mar 14, 2023
Setup lower runtime extension functions for host pipes. See also intel#5766 intel#5851 Host pipe sycl spec: intel#5838 (cherry picked from commit e4d513c) Zibai fixed the conflict.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To align with spec: #5838
Host pipe is a FIFO construct that provide links between elements of design that are accessed through read/write API. Host pipe is a pipe that links device kernel with host program.