From 60b26f1546cdafc6d7349e669dae3cea39c9c5c4 Mon Sep 17 00:00:00 2001 From: Andreas Sandberg Date: Mon, 27 Feb 2017 11:25:01 +0000 Subject: [PATCH] base: Refactor logging to make log level selection cleaner It's currently possible to change the log level in gem5 by tweaking a set of global variables. These variables are currently exposed to Python using SWIG. This mechanism is far from ideal for two reasons: First, changing the log level requires that the Python world enables or disables individual levels. Ideally, this should be a single call where a log level is selected. Second, exporting global variables is poorly supported by most Python frameworks. SWIG puts variables in their own namespace and PyBind doesn't seem to support it at all. This changeset refactors the logging code to create a more abstract interface. Each log level is associated with an instance of a Logger class. This class contains common functionality, an enable flag, and a verbose flag. Available LogLevels are described by the LogLevel class. Lower log levels are used for more critical messages (PANIC being level 0) and higher levels for less critical messages. The highest log level that is printed is controlled by calling Logger:setLevel(). Change-Id: I31e44299d242d953197a8e62679250c91d6ef776 Signed-off-by: Andreas Sandberg Reviewed-by: Gabor Dozsa Reviewed-by: Curtis Dunham Reviewed-by: Jason Lowe-Power --- src/base/misc.cc | 65 +++++------ src/base/misc.hh | 211 +++++++++++++++++++----------------- src/python/swig/core.i | 21 +++- tests/configs/switcheroo.py | 3 +- 4 files changed, 163 insertions(+), 137 deletions(-) diff --git a/src/base/misc.cc b/src/base/misc.cc index 644545db2..2392d51fd 100644 --- a/src/base/misc.cc +++ b/src/base/misc.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014 ARM Limited + * Copyright (c) 2014, 2017 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -43,26 +43,35 @@ #include "base/misc.hh" -#include -#include -#include +#include -#include "base/cprintf.hh" #include "base/hostinfo.hh" #include "base/output.hh" #include "base/trace.hh" #include "base/types.hh" #include "sim/core.hh" -using namespace std; -bool want_warn = true; -bool want_info = true; -bool want_hack = true; +Logger & +Logger::get(LogLevel ll) +{ + static std::array loggers{{ + new ExitLogger(std::cerr, "panic"), + new ExitLogger(std::cerr, "fatal"), + new Logger(std::cerr, "warn"), + new Logger(std::cerr, "info"), + new Logger(std::cerr, "hack"), + }}; -bool warn_verbose = false; -bool info_verbose = false; -bool hack_verbose = false; + return *loggers[ll]; +} + +void +Logger::setLevel(LogLevel ll) +{ + for (int i = 0; i < NUM_LOG_LEVELS; ++i) + get(LogLevel(i)).enabled = (i <= ll); +} static void newline_if_needed(std::ostream &stream, const char *format) @@ -78,34 +87,28 @@ newline_if_needed(std::ostream &stream, const char *format) } } -void -__exit_epilogue(int code, - const char *func, const char *file, int line, - const char *format) +Logger::Logger(std::ostream &_stream, const char *_prefix) + : enabled(true), verbose(false), stream(_stream), prefix(_prefix) { - newline_if_needed(std::cerr, format); - - ccprintf(std::cerr, - " @ tick %d\n" - "[%s:%s, line %d]\n" - "Memory Usage: %ld KBytes\n", - curTick(), func, file, line, memUsage()); - - if (code < 0) - abort(); - else - exit(code); } void -__base_message_epilogue(std::ostream &stream, bool verbose, - const char *func, const char *file, int line, +Logger::printEpilogue(const char *func, const char *file, int line, const char *format) { newline_if_needed(stream, format); if (verbose) { - ccprintf(stream, " @ cycle %d\n[%s:%s, line %d]\n", + ccprintf(stream, " @ tick %d\n[%s:%s, line %d]\n", curTick(), func, file, line); } } + +void +ExitLogger::printEpilogue(const char *func, const char *file, int line, + const char *format) +{ + Logger::printEpilogue(func, file, line, format); + + ccprintf(stream, "Memory Usage: %ld KBytes\n", memUsage()); +} diff --git a/src/base/misc.hh b/src/base/misc.hh index b62548eb2..58bd06be5 100644 --- a/src/base/misc.hh +++ b/src/base/misc.hh @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014 ARM Limited + * Copyright (c) 2014, 2017 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -47,6 +47,7 @@ #include #include +#include #include "base/compiler.hh" #include "base/cprintf.hh" @@ -55,44 +56,91 @@ #define __FUNCTION__ "how to fix me?" #endif -void __exit_epilogue(int code, - const char *func, const char *file, int line, - const char *format) M5_ATTR_NORETURN; - -// General exit message, these functions will never return and will -// either abort() if code is < 0 or exit with the code if >= 0 -template void -__exit_message(const char *prefix, int code, - const char *func, const char *file, int line, - const char *format, const Args &...args) M5_ATTR_NORETURN; -template void -__exit_message(const char *prefix, int code, - const char *func, const char *file, int line, - const std::string &format, const Args &...args) M5_ATTR_NORETURN; - -template void -__exit_message(const char *prefix, int code, - const char *func, const char *file, int line, - const char *format, const Args &...args) +class Logger { - std::cerr << prefix << ": "; - ccprintf(std::cerr, format, args...); + public: + enum LogLevel { + PANIC = 0, + FATAL, + WARN, + INFO, + HACK, + NUM_LOG_LEVELS, + }; - __exit_epilogue(code, func, file, line, format); -} + /** + * Set the active log level. + * + * All levels that are lower or equal to the selected log level + * will be activated. + * + * @param ll Maximum log level to print + */ + static void setLevel(LogLevel ll); -template void -__exit_message(const char *prefix, int code, - const char *func, const char *file, int line, - const std::string &format, const Args &...args) + /** + * Get a Logger corresponding to a specific log level + * + * @param ll Log level to access + * @return Reference to the requested logger + */ + static Logger &get(LogLevel ll); + + public: + Logger(std::ostream &stream, const char *prefix); + virtual ~Logger() {}; + + template void + print(const char *func, const char *file, int line, + const char *format, const Args &...args) + { + if (!enabled) + return; + + if (prefix) + stream << prefix << ": "; + ccprintf(stream, format, args...); + + printEpilogue(func, file, line, format); + } + + template void + print(const char *func, const char *file, int line, + const std::string &format, const Args &...args) + { + print(func, file, line, format.c_str(), args...); + } + + protected: + virtual void printEpilogue(const char *func, const char *file, int line, + const char *format); + + public: + bool enabled; + bool verbose; + + protected: + std::ostream &stream; + const char *prefix; +}; + +class ExitLogger : public Logger { - __exit_message(prefix, code, func, file, line, format.c_str(), - args...); -} + public: + using Logger::Logger; + + void printEpilogue(const char *func, const char *file, int line, + const char *format) override; +}; -#define exit_message(prefix, code, ...) \ - __exit_message(prefix, code, __FUNCTION__, __FILE__, __LINE__, \ - __VA_ARGS__) +#define exit_message(logger, code, ...) \ + do { \ + logger.print(__FUNCTION__, __FILE__, __LINE__, __VA_ARGS__); \ + if (code < 0) \ + ::abort(); \ + else \ + ::exit(code); \ + } while (0) // // This implements a cprintf based panic() function. panic() should @@ -101,7 +149,8 @@ __exit_message(const char *prefix, int code, // calls abort which can dump core or enter the debugger. // // -#define panic(...) exit_message("panic", -1, __VA_ARGS__) +#define panic(...) exit_message(::Logger::get(::Logger::PANIC), -1, \ + __VA_ARGS__) // // This implements a cprintf based fatal() function. fatal() should @@ -110,7 +159,8 @@ __exit_message(const char *prefix, int code, // etc.) and not a simulator bug. fatal() calls abort() like // panic() does. // -#define fatal(...) exit_message("fatal", -1, __VA_ARGS__) +#define fatal(...) exit_message(::Logger::get(::Logger::FATAL), 1, \ + __VA_ARGS__) /** * Conditional panic macro that checks the supplied condition and only panics @@ -120,10 +170,12 @@ __exit_message(const char *prefix, int code, * @param cond Condition that is checked; if true -> panic * @param ... Printf-based format string with arguments, extends printout. */ -#define panic_if(cond, ...) \ - do { \ - if ((cond)) \ - exit_message("panic condition "#cond" occurred", -1, __VA_ARGS__); \ +#define panic_if(cond, ...) \ + do { \ + if ((cond)) { \ + panic("panic condition " # cond " occurred: %s", \ + csprintf(__VA_ARGS__)); \ + } \ } while (0) @@ -136,41 +188,17 @@ __exit_message(const char *prefix, int code, * @param cond Condition that is checked; if true -> fatal * @param ... Printf-based format string with arguments, extends printout. */ -#define fatal_if(cond, ...) \ - do { \ - if ((cond)) \ - exit_message("fatal condition "#cond" occurred", 1, __VA_ARGS__); \ +#define fatal_if(cond, ...) \ + do { \ + if ((cond)) { \ + fatal("fatal condition " # cond " occurred: %s", \ + csprintf(__VA_ARGS__)); \ + } \ } while (0) -void -__base_message_epilogue(std::ostream &stream, bool verbose, - const char *func, const char *file, int line, - const char *format); - -template void -__base_message(std::ostream &stream, const char *prefix, bool verbose, - const char *func, const char *file, int line, - const char *format, const Args &...args) -{ - stream << prefix << ": "; - ccprintf(stream, format, args...); - - __base_message_epilogue(stream, verbose, func, file, line, format); -} - -template void -__base_message(std::ostream &stream, const char *prefix, bool verbose, - const char *func, const char *file, int line, - const std::string &format, const Args &...args) -{ - __base_message(stream, prefix, verbose, func, file, line, format.c_str(), - args...); -} - -#define base_message(stream, prefix, verbose, ...) \ - __base_message(stream, prefix, verbose, __FUNCTION__, __FILE__, __LINE__, \ - __VA_ARGS__) +#define base_message(logger, ...) \ + logger.print(__FUNCTION__, __FILE__, __LINE__, __VA_ARGS__) // Only print the message the first time this expression is // encountered. i.e. This doesn't check the string itself and @@ -186,37 +214,20 @@ __base_message(std::ostream &stream, const char *prefix, bool verbose, } \ } while (0) -#define cond_message(cond, ...) do { \ - if (cond) \ - base_message(__VA_ARGS__); \ - } while (0) - -#define cond_message_once(cond, ...) do { \ - static bool once = false; \ - if (!once && cond) { \ - base_message(__VA_ARGS__); \ - once = true; \ - } \ - } while (0) - - -extern bool want_warn, warn_verbose; -extern bool want_info, info_verbose; -extern bool want_hack, hack_verbose; #define warn(...) \ - cond_message(want_warn, std::cerr, "warn", warn_verbose, __VA_ARGS__) + base_message(::Logger::get(::Logger::WARN), __VA_ARGS__) #define inform(...) \ - cond_message(want_info, std::cout, "info", info_verbose, __VA_ARGS__) + base_message(::Logger::get(::Logger::INFO), __VA_ARGS__) #define hack(...) \ - cond_message(want_hack, std::cerr, "hack", hack_verbose, __VA_ARGS__) + base_message(::Logger::get(::Logger::HACK), __VA_ARGS__) #define warn_once(...) \ - cond_message_once(want_warn, std::cerr, "warn", warn_verbose, __VA_ARGS__) + base_message_once(::Logger::get(::Logger::WARN), __VA_ARGS__) #define inform_once(...) \ - cond_message_once(want_info, std::cout, "info", info_verbose, __VA_ARGS__) + base_message_once(::Logger::get(::Logger::INFO), __VA_ARGS__) #define hack_once(...) \ - cond_message_once(want_hack, std::cerr, "hack", hack_verbose, __VA_ARGS__) + base_message_once(::Logger::get(::Logger::HACK), __VA_ARGS__) /** * Conditional warning macro that checks the supplied condition and @@ -244,12 +255,10 @@ extern bool want_hack, hack_verbose; #ifdef NDEBUG #define chatty_assert(cond, ...) #else //!NDEBUG -#define chatty_assert(cond, ...) \ - do { \ - if (!(cond)) { \ - base_message(std::cerr, "assert("#cond") failing", 1, __VA_ARGS__); \ - assert(cond); \ - } \ +#define chatty_assert(cond, ...) \ + do { \ + if (!(cond)) \ + panic("assert(" # cond ") failed: %s", csprintf(__VA_ARGS__)); \ } while (0) #endif // NDEBUG #endif // __BASE_MISC_HH__ diff --git a/src/python/swig/core.i b/src/python/swig/core.i index 45337c3a5..8b734b940 100644 --- a/src/python/swig/core.i +++ b/src/python/swig/core.i @@ -92,7 +92,20 @@ void serializeAll(const std::string &cpt_dir); CheckpointIn *getCheckpoint(const std::string &cpt_dir); void unserializeGlobals(CheckpointIn &cp); -bool want_warn, warn_verbose; -bool want_info, info_verbose; -bool want_hack, hack_verbose; - +class Logger +{ + public: + enum LogLevel { + PANIC = 0, + FATAL, + WARN, + INFO, + HACK, + NUM_LOG_LEVELS, + }; + + static void setLevel(LogLevel ll); + + private: + Logger(); +}; diff --git a/tests/configs/switcheroo.py b/tests/configs/switcheroo.py index 43a20353e..c94dc7590 100644 --- a/tests/configs/switcheroo.py +++ b/tests/configs/switcheroo.py @@ -105,7 +105,8 @@ def run_test(root, switcher=None, freq=1000, verbose=False): # Suppress "Entering event queue" messages since we get tons of them. # Worse yet, they include the timestamp, which makes them highly # variable and unsuitable for comparing as test outputs. - _m5.core.cvar.want_info = verbose + if not verbose: + _m5.core.Logger.setLevel(_m5.core.Logger.WARN) # instantiate configuration m5.instantiate() -- 2.30.2