-
Notifications
You must be signed in to change notification settings - Fork 370
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
Overhaul error handling in the core framework #633
Draft
fgvanzee
wants to merge
8
commits into
master
Choose a base branch
from
error
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Details: - This commit makes changes pursuant to #479 that provides additional configure-time options for determining whether and how BLIS behaves when errors are detected at runtime. The options are presently not yet honored anywhere within the framework. However, this will change in the future.
Details: - Fully updated frame/0 in accordance to new error-handling APIs and error-checking policies. This includes: a. all functions that might possibly generate an error now return a value of type err_t. b. any such function in (a) that is called will have its return value captured and inspected for further return. - Updated about half of the files within frame/base for err_t handling. - Partially updated frame/thread, as necessary to given the updated err_t return values for other code included in this commit. A key omission was the thread decorators, which do not yet handle err_t values. - Added a new file, bli_error_macro_defs.h, of error-related macros. These macros conveniently capture the logic that should be executed in several common situations, including checking a locally-determined error code for failure (and acting accordingly) as well as checking whether the err_t return value from a recently-called function needs to be returned up the function stack (in leiu of completing execution of the current function normally). Not all of these functions are used in the changes introduced in this commit, but they represent most situations that I foresee needing going forward. - Re-indexed the err_t enum values so that BLIS_SUCCESS is assigned 0 and BLIS_FAILURE (that is, generic failure) is assigned -1, instead of -1 and -2, respectively. This beings the BLIS error code behavior into closer conformality with many other C and Linux functions and tools. - Defined a new errmode_t enum with two values -- BLIS_ERROR_RETURN and BLIS_ERROR_ABORT. - Defined new static inline functions in bli_param_macro_defs.h for distinguishing ind_t values (e.g. BLIS_NAT and BLIS_1M). Did the same for dir_t values (e.g. BLIS_FWD and BLIS_BWD). - Replaced all instances in BLIS of if ( rntm == NULL ) { bli_rntm_init_from_global( &rntm_l ); rntm = &rntm_l; } else { rntm_l = *rntm; rntm = &rntm_l; } with a call to a new static inline function that offers identical functionality: bli_rntm_init_if_null( &rntm, &rntm_l ); - Replaced all instances in BLIS of if ( cntx == NULL ) cntx = bli_gks_query_cntx(); with a call to a new static inline function that offers identical functionality: bli_gks_query_cntx_if_null( ( const cntx_t** )&cntx ); - Moved frame/base/cast/bli_castnzm.c and .h to an 'old' sub-directory. - Removed 'restrict' qualifier from cntx_t* argument to scalv and axpyf kernels in 'zen' kernel set. - Updated hardware auto-detection code to reflect updated function signature to bli_arch_string(). - Updated the output of 'configure --help' to correctly indicate that error checking is enabled by default. - Updated testsuite source files to conform to above changes. - Updated documentation to reflect updated function signatures, including removal of 'restrict' qualifier from cntx_t* and auxinfo_t* arguments to various kernels APIs.
Details: - Fixed a couple of compilation errors plus one logical error due to variable shadowing.
Details: - Promote bli_pba_rntm_set_pba() to a full function, rather than a static inline function, so that it can be exported for shared libraries. This seems better than the alternative of exporting the function bli_pba_query(), which, as far as I can see, end users should not need access to.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Going forward, this PR branch will implement the changes set forth in #479.
To start out with, only the new
configure
options are implemented. I'll incrementally update parts of the framework to (a) observe the new error checking flags and respond accordingly, and (b) returnerr_t
values up the call stack wherever possible.Thanks for your patience on this project, @jdiamondGitHub.