-
Notifications
You must be signed in to change notification settings - Fork 406
Overhaul error handling in the core framework #633
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
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 hidden or 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
configureoptions are implemented. I'll incrementally update parts of the framework to (a) observe the new error checking flags and respond accordingly, and (b) returnerr_tvalues up the call stack wherever possible.Thanks for your patience on this project, @jdiamondGitHub.