Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/6263
Change subject: misc: Rework the logging functions.
......................................................................
misc: Rework the logging functions.
Got rid of log "level" which wasn't actually used, and which also
didn't really make sense. We always want to see the output of warn,
fatal, etc, so it doesn't make sense to be able to turn those off. If
you have a message you do want to enable or disable, that's what the
DPRINTF functions are for. Also, the ordering of functions was
somewhat forced.
Removed the "verbose" switch which also wasn't used.
Replaced the "get(LogLevel)" function with a get for each level. The
parameter was always constant, so we can just call the right function
at the right time.
Made the "exit" behavior of panic/fatal a part of the logging
implementation so that it can be overridden, and corrected a comment
which said that both fatal and panic called ::abort().
Got rid of the printEpilogue function by reworking the print() methods.
The subclasses of Logger can now override a "log" function which takes
a composed message, letting the Logger class centralize how the message
is put together and leaving the actual output mechanism to the
subclass.
Unfortunately there wasn't a way to tell gcc that the panic/fatal
macros wouldn't return, so there needed to be an exit_helper wrapper
function which calls the actual logger exit function. That can be
marked as noreturn, unlike the virtual exit function. If the exit
function does return, the wrapper will call ::abort(), placating gcc
and ensuring that even if exit isn't implemented properly, exit_helper
will still not return. That also provides a handy default
implementation.
Change-Id: I66d0cebd59f1127db980f3b565dbdf60687d8862
---
M src/base/logging.cc
M src/base/logging.hh
M src/python/pybind11/core.cc
3 files changed, 118 insertions(+), 180 deletions(-)
diff --git a/src/base/logging.cc b/src/base/logging.cc
index cec9c83..adc3deb 100644
--- a/src/base/logging.cc
+++ b/src/base/logging.cc
@@ -43,80 +43,55 @@
#include "base/logging.hh"
-#include <array>
+#include <sstream>
#include "base/hostinfo.hh"
-#include "base/output.hh"
-#include "base/trace.hh"
-#include "base/types.hh"
-#include "sim/core.hh"
-void
-Logger::setLevel(LogLevel ll)
-{
- for (int i = 0; i < NUM_LOG_LEVELS; ++i)
- get(LogLevel(i)).enabled = (i <= ll);
-}
+namespace {
-static void
-newline_if_needed(std::ostream &stream, const char *format)
-{
- const size_t format_len(strlen(format));
-
- switch (format_len ? format[format_len - 1] : '\0') {
- case '\n':
- case '\r':
- break;
- default:
- stream << std::endl;
- }
-}
-
-Logger::Logger(std::ostream &_stream, const char *_prefix)
- : enabled(true), verbose(false), stream(_stream), prefix(_prefix)
-{
-}
-
-void
-Logger::printEpilogue(const char *func, const char *file, int line,
- const char *format)
-{
- newline_if_needed(stream, format);
-
- if (verbose) {
- ccprintf(stream, " @ tick %d\n[%s:%s, line %d]\n",
- curTick(), func, file, line);
- }
-}
-
-class ExitLogger : public Logger
+class NormalLogger : public Logger
{
public:
using Logger::Logger;
- void printEpilogue(const char *func, const char *file, int line,
- const char *format) override;
+ protected:
+ void log(const Loc &loc, std::string s) override { std::cerr << s; }
};
-void
-ExitLogger::printEpilogue(const char *func, const char *file, int line,
- const char *format)
+class ExitLogger : public NormalLogger
{
- Logger::printEpilogue(func, file, line, format);
+ public:
+ using NormalLogger::NormalLogger;
- ccprintf(stream, "Memory Usage: %ld KBytes\n", memUsage());
-}
+ protected:
+ void
+ log(const Loc &loc, std::string s) override
+ {
+ std::stringstream ss;
+ ccprintf(ss, "Memory Usage: %ld KBytes\n", memUsage());
+ NormalLogger::log(loc, s + ss.str());
+ }
+};
-Logger &
-Logger::get(LogLevel ll)
+class FatalLogger : public ExitLogger
{
- static std::array<Logger *, NUM_LOG_LEVELS> 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"),
- }};
+ public:
+ using ExitLogger::ExitLogger;
- return *loggers[ll];
-}
+ protected:
+ void exit() override { ::exit(1); }
+};
+
+ExitLogger panicLogger("panic: ");
+FatalLogger fatalLogger("fatal: ");
+NormalLogger warnLogger("warn: ");
+NormalLogger infoLogger("info: ");
+NormalLogger hackLogger("hack: ");
+
+} // anonymous namespace
+
+Logger &Logger::getPanic() { return panicLogger; }
+Logger &Logger::getFatal() { return fatalLogger; }
+Logger &Logger::getWarn() { return warnLogger; }
+Logger &Logger::getInfo() { return infoLogger; }
+Logger &Logger::getHack() { return hackLogger; }
diff --git a/src/base/logging.hh b/src/base/logging.hh
index b0a9b0c..584eb74 100644
--- a/src/base/logging.hh
+++ b/src/base/logging.hh
@@ -46,113 +46,109 @@
#define __BASE_LOGGING_HH__
#include <cassert>
-#include <cstdlib>
-#include <iostream>
+#include <sstream>
#include <utility>
#include "base/compiler.hh"
#include "base/cprintf.hh"
-#if defined(__SUNPRO_CC)
-#define __FUNCTION__ "how to fix me?"
-#endif
-
class Logger
{
public:
- enum LogLevel {
- PANIC = 0,
- FATAL,
- WARN,
- INFO,
- HACK,
- NUM_LOG_LEVELS,
+ // Get a Logger for the specified type of message.
+ static Logger &getPanic();
+ static Logger &getFatal();
+ static Logger &getWarn();
+ static Logger &getInfo();
+ static Logger &getHack();
+
+ struct Loc
+ {
+ Loc(const char *file, int line) : file(file), line(line) {}
+ const char *file;
+ int line;
};
- /**
- * 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);
-
- /**
- * 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);
+ Logger(const char *prefix) : prefix(prefix) { assert(prefix); }
virtual ~Logger() {};
- template<typename ...Args> void
- print(const char *func, const char *file, int line,
- const char *format, const Args &...args)
+ void
+ print(const Loc &loc, const std::string &str)
{
- if (!enabled)
- return;
-
- if (prefix)
- stream << prefix << ": ";
- ccprintf(stream, format, args...);
-
- printEpilogue(func, file, line, format);
+ std::stringstream ss;
+ ss << prefix << str;
+ if (str.length() && str.back() != '\n' && str.back() != '\r')
+ ss << std::endl;
+ log(loc, ss.str());
}
template<typename ...Args> void
- print(const char *func, const char *file, int line,
- const std::string &format, const Args &...args)
+ print(const Loc &loc, const char *format, const Args &...args)
{
- print(func, file, line, format.c_str(), args...);
+ std::stringstream ss;
+ ccprintf(ss, format, args...);
+ print(loc, ss.str());
}
- protected:
- virtual void printEpilogue(const char *func, const char *file, int
line,
- const char *format);
+ template<typename ...Args> void
+ print(const Loc &loc, const std::string &format, const Args &...args)
+ {
+ print(loc, format.c_str(), args...);
+ }
- public:
- bool enabled;
- bool verbose;
+ // This helper is necessary since noreturn isn't inherited by virtual
+ // functions, and gcc will get mad if a function calls panic and then
+ // doesn't return.
+ void exit_helper() M5_ATTR_NORETURN { exit(); ::abort(); }
protected:
- std::ostream &stream;
+ virtual void log(const Loc &loc, std::string s) = 0;
+ virtual void exit() { /* Fall through to the abort in exit_helper. */ }
+
const char *prefix;
};
-#define exit_message(logger, code, ...) \
- do { \
- logger.print(__FUNCTION__, __FILE__, __LINE__, __VA_ARGS__); \
- if (code < 0) \
- ::abort(); \
- else \
- ::exit(code); \
+
+#define base_message(logger, ...) \
+ logger.print(::Logger::Loc(__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
+ * prevent duplicate strings, this prevents the statement from
+ * happening more than once. So, even if the arguments change and that
+ * would have resulted in a different message thoes messages would be
+ * supressed.
+ */
+#define base_message_once(...) do { \
+ static bool once = false; \
+ if (!once) { \
+ base_message(__VA_ARGS__); \
+ once = true; \
+ } \
} while (0)
-//
-// This implements a cprintf based panic() function. panic() should
-// be called when something happens that should never ever happen
-// regardless of what the user does (i.e., an acutal m5 bug). panic()
-// calls abort which can dump core or enter the debugger.
-//
-//
-#define panic(...) exit_message(::Logger::get(::Logger::PANIC), -1, \
- __VA_ARGS__)
+#define exit_message(logger, ...) \
+ do { \
+ base_message(logger, __VA_ARGS__); \
+ logger.exit_helper(); \
+ } while (0)
-//
-// This implements a cprintf based fatal() function. fatal() should
-// be called when the simulation cannot continue due to some condition
-// that is the user's fault (bad configuration, invalid arguments,
-// etc.) and not a simulator bug. fatal() calls abort() like
-// panic() does.
-//
-#define fatal(...) exit_message(::Logger::get(::Logger::FATAL), 1, \
- __VA_ARGS__)
+/**
+ * This implements a cprintf based panic() function. panic() should
+ * be called when something happens that should never ever happen
+ * regardless of what the user does (i.e., an acutal m5 bug). panic()
+ * might call abort which can dump core or enter the debugger.
+ */
+#define panic(...) exit_message(::Logger::getPanic(), __VA_ARGS__)
+
+/**
+ * This implements a cprintf based fatal() function. fatal() should
+ * be called when the simulation cannot continue due to some condition
+ * that is the user's fault (bad configuration, invalid arguments,
+ * etc.) and not a simulator bug. fatal() might call exit, unlike panic().
+ */
+#define fatal(...) exit_message(::Logger::getFatal(), __VA_ARGS__)
/**
* Conditional panic macro that checks the supplied condition and only
panics
@@ -189,37 +185,13 @@
} while (0)
-#define base_message(logger, ...) \
- logger.print(__FUNCTION__, __FILE__, __LINE__, __VA_ARGS__)
+#define warn(...) base_message(::Logger::getWarn(), __VA_ARGS__)
+#define inform(...) base_message(::Logger::getInfo(), __VA_ARGS__)
+#define hack(...) base_message(::Logger::getHack(), __VA_ARGS__)
-// Only print the message the first time this expression is
-// encountered. i.e. This doesn't check the string itself and
-// prevent duplicate strings, this prevents the statement from
-// happening more than once. So, even if the arguments change and that
-// would have resulted in a different message thoes messages would be
-// supressed.
-#define base_message_once(...) do { \
- static bool once = false; \
- if (!once) { \
- base_message(__VA_ARGS__); \
- once = true; \
- } \
- } while (0)
-
-
-#define warn(...) \
- base_message(::Logger::get(::Logger::WARN), __VA_ARGS__)
-#define inform(...) \
- base_message(::Logger::get(::Logger::INFO), __VA_ARGS__)
-#define hack(...) \
- base_message(::Logger::get(::Logger::HACK), __VA_ARGS__)
-
-#define warn_once(...) \
- base_message_once(::Logger::get(::Logger::WARN), __VA_ARGS__)
-#define inform_once(...) \
- base_message_once(::Logger::get(::Logger::INFO), __VA_ARGS__)
-#define hack_once(...) \
- base_message_once(::Logger::get(::Logger::HACK), __VA_ARGS__)
+#define warn_once(...) base_message_once(::Logger::getWarn(), __VA_ARGS__)
+#define inform_once(...) base_message_once(::Logger::getInfo(),
__VA_ARGS__)
+#define hack_once(...) base_message_once(::Logger::getHack(), __VA_ARGS__)
/**
* Conditional warning macro that checks the supplied condition and
diff --git a/src/python/pybind11/core.cc b/src/python/pybind11/core.cc
index 41eeed2..29db200 100644
--- a/src/python/pybind11/core.cc
+++ b/src/python/pybind11/core.cc
@@ -226,16 +226,7 @@
.def_readwrite("tm_isdst", &tm::tm_isdst)
;
- py::enum_<Logger::LogLevel>(m_core, "LogLevel")
- .value("PANIC", Logger::PANIC)
- .value("FATAL", Logger::FATAL)
- .value("WARN", Logger::WARN)
- .value("INFO", Logger::INFO)
- .value("HACK", Logger::HACK)
- ;
-
m_core
- .def("setLogLevel", &Logger::setLevel)
.def("setOutputDir", &setOutputDir)
.def("doExitCleanup", &doExitCleanup)
--
To view, visit https://gem5-review.googlesource.com/6263
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I66d0cebd59f1127db980f3b565dbdf60687d8862
Gerrit-Change-Number: 6263
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <[email protected]>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
