[BOLT][NFC] Refactor logging and error handling #81082
Closed
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
Make core BOLT functionality more friendly to being used as a library instead of in our standalone driver llvm-bolt. To accomplish this, we augment BinaryContext with journaling streams that are to be used by most BOLT code whenever something needs to be logged to the screen. Users of the library can decide if logs should be printed to a file, no file or to the screen, as before. To illustrate this, this patch adds a new option --log-file that allows the user to redirect BOLT logging to a file on disk or completely hide it by using --log-file=/dev/null. Future BOLT code should now use BinaryContext::outs() for printing important messages instead of llvm::outs(). A new test log.test enforces this by verifying that no strings are print to screen once the --log-file option is used. We also refactor old error handling code that would directly call exit(1) inside BOLT libraries whenever a serious problem occurred. To overcome this, we add a new class BOLTError and auxiliary functions createFatalBOLTError() and createNonFatalBOLTError() that allows BOLT libs to bubble up the problem to the caller by using the Error class as a return type (or Expected). To easily handle problems as before (by quitting with exit(1)), callers can now use BinaryContext::logBOLTErrorsAndQuitOnFatal(Error) whenever code needs to deal with BOLT errors. To test this, we have fatal.s that checks we are correctly quitting and printing a fatal error to the screen. Because this is a significant change by itself, not all code was yet ported. Code from Profiler libs (DataAggregator and friends) still print errors directly to screen.
This was referenced Feb 12, 2024
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.
Make core BOLT functionality more friendly to being used as a library instead of in our standalone driver llvm-bolt. To accomplish this, we augment BinaryContext with journaling streams that are to be used by most BOLT code whenever something needs to be logged to the screen. Users of the library can decide if logs should be printed to a file, no file or to the screen, as before. To illustrate this, this patch adds a new option --log-file that allows the user to redirect BOLT logging to a file on disk or completely hide it by using --log-file=/dev/null. Future BOLT code should now use BinaryContext::outs() for printing important messages instead of llvm::outs(). A new test log.test enforces this by verifying that no strings are print to screen once the --log-file option is used.
We also refactor old error handling code that would directly call exit(1) inside BOLT libraries whenever a serious problem occurred. To overcome this, we add a new class BOLTError and auxiliary functions createFatalBOLTError() and createNonFatalBOLTError() that allows BOLT libs to bubble up the problem to the caller by using the Error class as a return type (or Expected). To easily handle problems as before (by quitting with exit(1)), callers can now use
BinaryContext::logBOLTErrorsAndQuitOnFatal(Error) whenever code needs to deal with BOLT errors. To test this, we have fatal.s that checks we are correctly quitting and printing a fatal error to the screen.
Because this is a significant change by itself, not all code was yet ported. Code from Profiler libs (DataAggregator and friends) still print errors directly to screen.