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

Introduce a logging method wrapper for importVerifyModule #14724

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tomdol
Copy link
Contributor

@tomdol tomdol commented Feb 20, 2025

This is a proposal of a change of the underlying behavior of the ImporterEx::importVerifyModule method.

It's related to the comments in the NPU_Compiler/pull/20425 pull request where the switch to the current version of this method has been rejected.

The proposal changes the error handling of importVerifyModule from writing directly to std::cerr to throwing an exception instead. This way different users of this method can decide how they want to handle the error.

At the same time this PR contains a helper function which maintains the behavior in this repository - the verifier error, if occurs, will be logged to std::cerr. The code adaptation in this repository is also minimal with this approach.

Related issue: NPU_Compiler/issues/20383
Related PR: NPU_Compiler/pull/20425

@tomdol tomdol requested a review from seanshpark February 20, 2025 10:55
@tomdol
Copy link
Contributor Author

tomdol commented Feb 20, 2025

The suggested change can be contained in the ImporterEx class too, the base throwing functionality can be kept in one method while another (new) method can catch the exception and log to cerr. This way the impact on the existing call sites of importVerifyModule can be reduced even further.

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.

1 participant