Skip to content

Initial draft of data flow pipes extension proposal, and move files into pipes subfolder - #635

Merged
bader merged 5 commits into
intel:syclfrom
mkinsner:pipes_extension
Nov 14, 2019
Merged

bader merged 5 commits into
intel:syclfrom
mkinsner:pipes_extension

Conversation

@mkinsner

Copy link
Copy Markdown

No description provided.

@keryell

keryell commented Sep 17, 2019

Copy link
Copy Markdown
Contributor

There was also #318 related to this.

@keryell keryell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great proposal!
Here is my review on the first half of the proposal.

Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated

@keryell keryell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the great proposal.
That looks good on my side, just with a few typos to fix.
The most important point for me is to stream-line the map/unmap API.

Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
@bader

bader commented Nov 7, 2019

Copy link
Copy Markdown
Contributor

@mkinsner, please, address comments from @keryell.

@mkinsner

Copy link
Copy Markdown
Author

Thank you for the great proposal.
That looks good on my side, just with a few typos to fix.
The most important point for me is to stream-line the map/unmap API.

Thanks for the review and feedback @keryell ! I've incorporated almost all of it. Three larger items are tracked for the future in: #832

@mkinsner

Copy link
Copy Markdown
Author

@bader I believe that this extension revision is ready to merge.

@bader

bader commented Nov 13, 2019

Copy link
Copy Markdown
Contributor

@bader I believe that this extension revision is ready to merge.

Great! Thank you!
Are you okay to wait for @keryell approve before the merge?

@mkinsner

Copy link
Copy Markdown
Author

@bader I believe that this extension revision is ready to merge.

Great! Thank you!
Are you okay to wait for @keryell approve before the merge?

I'd prefer to merge this week. We can wait a day or two to see whether @keryell is online and has bandwidth to approve.

@keryell keryell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small typo.

Comment thread sycl/doc/extensions/DataFlowPipes/data_flow_pipes.asciidoc Outdated
Signed-off-by: Michael Kinsner <[email protected]>
keryell
keryell previously approved these changes Nov 14, 2019

@keryell keryell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good.

@MrSidims

Copy link
Copy Markdown
Contributor

Just one nit:
I think that SYCLPipesUsage.rst can be removed at all.

Signed-off-by: Michael Kinsner <[email protected]>
@bader
bader merged commit ee2b482 into intel:sycl Nov 14, 2019
@mkinsner
mkinsner deleted the pipes_extension branch November 14, 2019 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants