Skip to content
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

[onert] Propagate shared memory operand indexes to cpu backend #14230

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mbencer
Copy link
Contributor

@mbencer mbencer commented Oct 16, 2024

This commit adds propagation of shared memory operand indexes to cpu backend. Note that the propagated indexes map is not filled yet.

ONE-DCO-1.0-Signed-off-by: Mateusz Bencer [email protected]

Draft: #14057

This commit adds propagation of shared memory operand indexes to cpu backend.
Note that the propagated indexes map is not filled yet.

ONE-DCO-1.0-Signed-off-by: Mateusz Bencer [email protected]
Copy link
Contributor

@zetwhite zetwhite left a comment

Choose a reason for hiding this comment

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

This PR changes the interface between the core and the backend. The code seems fine, but I'm not 100% sure about the direction (or details). It's complicated for me to review this.

And I'm also worried that you might have to work twice because there are different opinions between me and other members of onert. For this part, I would like to hear others' opinions first. (maybe @ragmani or @hseok-oh)

@ragmani
Copy link
Contributor

ragmani commented Oct 31, 2024

This PR changes the interface between the core and the backend. The code seems fine, but I'm not 100% sure about the direction (or details). It's complicated for me to review this.

@zetwhite
I couldn't find the part that changes the interface. Could you explain in more detail what you worried about?

@zetwhite
Copy link
Contributor

zetwhite commented Nov 1, 2024

@ragmani

I couldn't find the part that changes the interface.

I thought that virtual BackendContext::sharedMemoryOperandIndexes() is newly introduced.

Could you explain in more detail what you worried about?

I'm fine with virtual BackendContext::sharedMemoryOperandIndexes() :)

But I'm not sure why builtin::TensorBuilder has to be changed in this PR. I thought basic::TensorBuilder.h and builtin::TensorBuilder had no dependency.

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.

3 participants