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] Introduce ExtraTensor #13576

Merged
merged 3 commits into from
Aug 2, 2024
Merged

Conversation

zetwhite
Copy link
Contributor

@zetwhite zetwhite commented Aug 1, 2024

This PR introduces ExtraTensor.
ExtraTensor means that the tensor is only used by one operation.

ONE-DCO-1.0-Signed-off-by: seunghui youn sseung.youn@samsung.com
draft : #13486
related : #13282

@zetwhite zetwhite requested a review from a team August 1, 2024 10:23
@zetwhite zetwhite assigned zetwhite and unassigned zetwhite Aug 1, 2024
@zetwhite zetwhite added the approval: 2 Require at least 2 approvals label Aug 1, 2024
This PR introduces ExtraTensor.
ExtraTensor means that the tensor is only used by one operation.

ONE-DCO-1.0-Signed-off-by: seunghui youn <sseung.youn@samsung.com>
@zetwhite zetwhite added the PR/ready for review It is ready to review. Please review it. label Aug 1, 2024
@ragmani
Copy link
Contributor

ragmani commented Aug 1, 2024

According to https://en.cppreference.com/w/cpp/language/rule_of_three,


Rule of three

If a class requires a user-defined destructor, a user-defined copy constructor, or a user-defined copy assignment operator, it almost certainly requires all three.

Rule of five

Because the presence of a user-defined (or = default or = delete declared) destructor, copy-constructor, or copy-assignment operator prevents implicit definition of the move constructor and the move assignment operator, any class for which move semantics are desirable, has to declare all five special member functions:


Could you follow the rule of five if move semantics are desirable for the class?

@ragmani
Copy link
Contributor

ragmani commented Aug 2, 2024

Could you follow the rule of five if move semantics are desirable for the class?

I read the code wrong as user-defined copy constructor. You don't need to follow the rule unless there are user-defined special functions.

@zetwhite
Copy link
Contributor Author

zetwhite commented Aug 2, 2024

You don't need to follow the rule unless there are user-defined special functions.

@ragmani If so I'd like to get a review of this PR as it is.

(+) About basic::Tensor's move ctor and move operators (what we talked on offline), I'd like to make it a different PR.

ragmani
ragmani previously approved these changes Aug 2, 2024
Copy link
Contributor

@ragmani ragmani left a comment

Choose a reason for hiding this comment

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

LGTM

@zetwhite zetwhite requested a review from a team August 2, 2024 06:39
zetwhite

This comment was marked as off-topic.

@zetwhite zetwhite requested a review from ragmani August 2, 2024 07:05
Copy link
Contributor

@hseok-oh hseok-oh left a comment

Choose a reason for hiding this comment

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

LGTM

@hseok-oh hseok-oh merged commit 5457886 into Samsung:master Aug 2, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: 2 Require at least 2 approvals PR/ready for review It is ready to review. Please review it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants