Skip to content

[Android NNAPI EP] change most of the exceptions to return Status#4701

Merged
guoyu-wang merged 8 commits into
masterfrom
nnapi_return_status
Aug 5, 2020
Merged

[Android NNAPI EP] change most of the exceptions to return Status#4701
guoyu-wang merged 8 commits into
masterfrom
nnapi_return_status

Conversation

@guoyu-wang
Copy link
Copy Markdown
Contributor

Description: change most of the exceptions in NNAPI EP to return Status

Motivation and Context

  • Using return Status may potentially reduce the binary size of the package
  • If we choose to not use exception for Android (to reduce binary size), return Status will give app a way of exit gracefully
  • In case of an NNAPI EP execution failure (rarely)m, ORT may be able to fall back to CPU

@guoyu-wang guoyu-wang requested a review from skottmckay August 4, 2020 00:22
@guoyu-wang guoyu-wang requested a review from a team as a code owner August 4, 2020 00:22
@skottmckay
Copy link
Copy Markdown
Contributor

skottmckay commented Aug 4, 2020

Is there a compile option that checks for unused return values to make sure there's nowhere we're accidentally ignoring one? #Resolved

skottmckay
skottmckay previously approved these changes Aug 4, 2020
Copy link
Copy Markdown
Contributor

@skottmckay skottmckay left a comment

Choose a reason for hiding this comment

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

:shipit:

@guoyu-wang
Copy link
Copy Markdown
Contributor Author

Seems the -Wunused-result option is by default on, seems it will not work for non-POD type, here is the gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=38172
Unless we bump to c++17 and use [[nodiscard]] attribute


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

@guoyu-wang
Copy link
Copy Markdown
Contributor Author

Actually I found we have ORT_MUST_USE_RESULT to check if the return Status is used, added it in the latest iter


In reply to: 668783113 [](ancestors = 668783113,668400618)

Copy link
Copy Markdown
Contributor

@skottmckay skottmckay left a comment

Choose a reason for hiding this comment

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

:shipit:

@guoyu-wang guoyu-wang merged commit 0933148 into master Aug 5, 2020
@guoyu-wang guoyu-wang deleted the nnapi_return_status branch August 5, 2020 04:37
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