Skip to content

Slice Op: remove the template code#155

Closed
HectorSVC wants to merge 2 commits intomasterfrom
hecli/slice_cpu
Closed

Slice Op: remove the template code#155
HectorSVC wants to merge 2 commits intomasterfrom
hecli/slice_cpu

Conversation

@HectorSVC
Copy link
Contributor

No description provided.

const auto* output_end = output + output_shape.Size();
auto output_tensor_ptr = ctx->Output(0, output_shape);
int64_t output_size = output_shape.Size();
if (0 == output_size) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't the below code just do nothing if output_size is zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's not guaranteed, so return early here.


In reply to: 241155600 [](ancestors = 241155600)

Copy link
Contributor

Choose a reason for hiding this comment

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

You should put a comment saying that we must do this check then, then it's clear it's not just an optimization.

Copy link
Contributor

@RyanUnderhill RyanUnderhill left a comment

Choose a reason for hiding this comment

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

We talked offline, but writing it down here so we don't forget. Can you compile it to see the before/after of the file size to see how many bytes we save by this new version?

@HectorSVC HectorSVC closed this Feb 9, 2019
@pranavsharma pranavsharma deleted the hecli/slice_cpu branch April 2, 2019 07:55
@pranavsharma pranavsharma restored the hecli/slice_cpu branch April 2, 2019 07:56
@pranavsharma pranavsharma deleted the hecli/slice_cpu branch April 2, 2019 07: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.

2 participants