[onert] Propagate shared memory operand indexes to cpu backend

[mbencer]

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

[zetwhite]

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]

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]

@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.h had no dependency.

[ragmani]

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

I realized what interface changed.

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.

I guess sharedMemoryOperandIndexes was introduced because of the basic::TensorBuilder.h modification.
@mbencer I would like to implement the In-place functionality as an independent module(like function without modifying the existing for loop) but I don't have time to look through your code carefully now. Could you copy your changes into cpu backend instead of the basic::TensorBuilder.h modification although it would cause code duplication? If so, you could remove BackendContext::sharedMemoryOperandIndexes().

[mbencer]

@zetwhite @ragmani

I thought basic::TensorBuilder.h and builtin::TensorBuilder.h had no dependency

Unfortunately, they are dependent via static polymorphism (despite the lack common interface) in genTensors where ctx a passed as an template argument

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

I see, after modification of BackendContextHelpers.h I was able to revert changes in builtin.

I would like to implement the In-place functionality as an independent module(like function without modifying the existing for loop)

As I understand you propose to calculate indexes of shared memory operands during each usage? I see advantages of such approach (limited scope of changes) but please also review the current approach - note that shared_memory_operand_idx param is passed to genTensors and initConsts but not used yet (postponed to the separate PR).

If so, you could remove BackendContext::sharedMemoryOperandIndexes()

The scope is limited now to backend/cpu.

[zetwhite]

LGTM

[mbencer]

FYI: I will be on vacation until 26 (Nov) with excluding Wed (20 Nov) - in this day I am going to work. Please expect delays in communication ;-)

[ragmani]

LGTM