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

[luci-interpreter] Add StaticMemoryManager #7735

Merged
merged 3 commits into from
Sep 28, 2021

Conversation

BalyshevArtem
Copy link
Contributor

This pr adds static memory manager to luci-interpreter.

issue: #7522

draft: #7573

ONE-DCO-1.0-Signed-off-by: Artem Balyshev a.balyshev@partner.samsung.com

binarman
binarman previously approved these changes Sep 27, 2021
Copy link
Contributor

@binarman binarman left a comment

Choose a reason for hiding this comment

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

LGTM

m-bronnikov
m-bronnikov previously approved these changes Sep 27, 2021
Copy link
Contributor

@m-bronnikov m-bronnikov left a comment

Choose a reason for hiding this comment

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

LGTM

public:
StaticMemoryManager() = delete;

explicit StaticMemoryManager(uint8_t *buffer_ptr) { _buffer_ptr = buffer_ptr; }
Copy link
Contributor

Choose a reason for hiding this comment

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

(optional)

Suggested change
explicit StaticMemoryManager(uint8_t *buffer_ptr) { _buffer_ptr = buffer_ptr; }
explicit StaticMemoryManager(uint8_t *buffer_ptr) : _buffer_ptr(buffer_ptr) { /* Do nothing */ }

Copy link
Contributor

Choose a reason for hiding this comment

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

Need to run nnas format, I think this line will be wrapped

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to run nnas format, I think this line will be wrapped
@binarman @m-bronnikov
I did nnas format and in result it divided the figured brackets on rows

Balyshev Artem added 2 commits September 28, 2021 12:03
This commit adds static memory manager.

ONE-DCO-1.0-Signed-off-by: Artem Balyshev a.balyshev@partner.samsung.com
This commit deletes destructor and fixes comment in StaticMemoryManager

ONE-DCO-1.0-Signed-off-by: Artem Balyshev a.balyshev@partner.samsung.com
@nnfw-bot
Copy link
Collaborator

format-checker failed, please fix format by following commands (need clang-format 3.9):

  • ./nnas format
  • git commit -a -s --amend

Or apply format.patch using following commands:

  • git apply format.patch
  • git commit -a -s --amend

Or there may be files having CRLF as newline. Please use LF.

To re-run this test, leave a comment @nnfw-bot test format-checker

This commit fixes constructor and remove redundant actions in allocate method.

ONE-DCO-1.0-Signed-off-by: Artem Balyshev a.balyshev@partner.samsung.com

Signed-off-by: Artem Balyshev <a.balyshev@partner.samsung.com>
binarman
binarman previously approved these changes Sep 28, 2021
public:
StaticMemoryManager() = delete;

explicit StaticMemoryManager(uint8_t *buffer_ptr) { _buffer_ptr = buffer_ptr; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to run nnas format, I think this line will be wrapped

Copy link
Contributor

@m-bronnikov m-bronnikov left a comment

Choose a reason for hiding this comment

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

LGTM

StaticMemoryManager() = delete;

explicit StaticMemoryManager(uint8_t *buffer_ptr) : _buffer_ptr(buffer_ptr)
{ /* Do nothing */
Copy link
Contributor

@m-bronnikov m-bronnikov Sep 28, 2021

Choose a reason for hiding this comment

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

Looks strange, but Im okay with it =)

@binarman binarman merged commit cf46ac3 into Samsung:master Sep 28, 2021
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