From 38672ccfe4642ddd4e2265db928374d158afaf07 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Wed, 8 Apr 2020 23:55:14 -0700 Subject: [PATCH] util: Pull "usage()" out of the call types in the m5 utility. Also pull common implementations of some call type methods into the base class, and make disappearing call types clean themselves up to make the test a little simpler and less error prone. Change-Id: Ie178fe02d41587647ddc90a084d1d1142b84dde9 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27687 Reviewed-by: Giacomo Travaglini Reviewed-by: Daniel Carvalho Maintainer: Gabe Black Tested-by: kokoro --- util/m5/src/SConscript.native | 2 +- util/m5/src/call_type.cc | 46 ++++++++++++----- util/m5/src/call_type.hh | 32 +++++++++--- util/m5/src/call_type.test.cc | 94 ++++++++++++++--------------------- util/m5/src/call_type/addr.cc | 25 +++++----- util/m5/src/call_type/inst.cc | 13 +---- util/m5/src/call_type/semi.cc | 13 +---- util/m5/src/m5.cc | 6 ++- 8 files changed, 118 insertions(+), 113 deletions(-) diff --git a/util/m5/src/SConscript.native b/util/m5/src/SConscript.native index e4897ce91..b9ae6bcc7 100644 --- a/util/m5/src/SConscript.native +++ b/util/m5/src/SConscript.native @@ -28,7 +28,7 @@ Import('*') env.Append(CPPPATH=Dir('.')) env.GTest('args') -env.GTest('call_type') +env.GTest('call_type', 'call_type.test.cc', 'call_type.cc', 'args.cc') env.GTest('command', 'command.test.cc', 'command.cc', 'args.cc') command_tests = env.SConscript('command/SConscript.native', exports='env') diff --git a/util/m5/src/call_type.cc b/util/m5/src/call_type.cc index 5deaae1c5..839a92c4e 100644 --- a/util/m5/src/call_type.cc +++ b/util/m5/src/call_type.cc @@ -28,40 +28,60 @@ #include #include +#include "args.hh" #include "call_type.hh" -std::vector & -CallType::allTypes() +std::map & +CallType::map() { - static std::vector all; + static std::map all; return all; } -CallType & +CallType::CheckArgsResult +CallType::checkArgs(Args &args) +{ + if (args.size() && args[0] == "--" + name) { + args.pop(); + return CheckArgsResult::Match; + } + return CheckArgsResult::NoMatch; +} + +CallType * CallType::detect(Args &args) { CallType *def = nullptr; - for (auto *ct: allTypes()) { - if (ct->checkArgs(args)) { - ct->init(); - return *ct; + for (auto p: map()) { + auto &ct = p.second; + if (ct.isDefault()) + def = &ct; + auto result = ct.checkArgs(args); + switch (result) { + case CheckArgsResult::Match: + ct.init(); + return &ct; + case CheckArgsResult::NoMatch: + continue; + case CheckArgsResult::Usage: + return nullptr; + default: + assert(!"Bad checkArgs result"); } - if (ct->isDefault()) - def = ct; } assert(def); def->init(); - return *def; + return def; } std::string CallType::usageSummary() { std::string summary = ""; - for (auto *ct: allTypes()) - summary += ct->formattedUsage(); + for (auto p: map()) + summary += p.second.formattedUsage(); return summary; } diff --git a/util/m5/src/call_type.hh b/util/m5/src/call_type.hh index 4b2b3d120..9744d3d0c 100644 --- a/util/m5/src/call_type.hh +++ b/util/m5/src/call_type.hh @@ -29,29 +29,49 @@ #define __CALL_TYPE_HH__ #include +#include #include -#include +#include class Args; class DispatchTable; class CallType { + public: + enum class CheckArgsResult { + Match, + NoMatch, + Usage + }; + protected: + const std::string name; + virtual bool isDefault() const = 0; - virtual bool checkArgs(Args &args) = 0; + virtual CheckArgsResult checkArgs(Args &args); virtual void init() {} - static std::vector &allTypes(); + static std::map &map(); - virtual void printBrief(std::ostream &os) const = 0; + virtual void printBrief(std::ostream &os) const { os << "--" << name; } virtual void printDesc(std::ostream &os) const = 0; std::string formattedUsage() const; public: - CallType() { allTypes().push_back(this); } + CallType(const std::string &_name) : name(_name) + { + map().emplace(std::piecewise_construct, + std::forward_as_tuple(std::string(_name)), + std::forward_as_tuple(*this)); + } + + ~CallType() + { + map().erase(name); + } - static CallType &detect(Args &args); + static CallType *detect(Args &args); static std::string usageSummary(); virtual const DispatchTable &getDispatch() const = 0; diff --git a/util/m5/src/call_type.test.cc b/util/m5/src/call_type.test.cc index 0aad8439b..daf6ddf69 100644 --- a/util/m5/src/call_type.test.cc +++ b/util/m5/src/call_type.test.cc @@ -30,18 +30,17 @@ // For EXPECT_THAT and HasSubstr #include +#include "args.hh" #include "call_type.hh" // Simple substitute definitions for Args and DispatchTable, with an int so // we can tell instances apart. -class Args { public: int i; }; class DispatchTable { public: int i; }; class TestCallType : public CallType { protected: bool isDefault() const override { return testIsDefault; } - bool checkArgs(Args &args) override { return args.i == testAcceptArgs; } void init() override { @@ -52,7 +51,9 @@ class TestCallType : public CallType void printBrief(std::ostream &os) const override { os << testBrief; } void printDesc(std::ostream &os) const override { os << testDesc; } public: - static std::vector &testGetAllTypes() { return allTypes(); } + TestCallType(const std::string &_name) : CallType(_name) {} + + static std::map &testGetMap() { return map(); } // Usage strings to return. std::string testBrief; @@ -62,9 +63,6 @@ class TestCallType : public CallType DispatchTable testDt = { 0 }; const DispatchTable &getDispatch() const override { return testDt; } - // If an Args has this value, accept it. - int testAcceptArgs = 0; - // Whether this call type should be considered default. bool testIsDefault = false; @@ -74,124 +72,112 @@ class TestCallType : public CallType TEST(CallTypeTest, Constructor) { - auto &all_types = TestCallType::testGetAllTypes(); + auto &map = TestCallType::testGetMap(); // There should be no call types yet. - EXPECT_EQ(all_types.size(), 0); + EXPECT_EQ(map.size(), 0); // Create one. - TestCallType test_ct; + TestCallType test_ct("test_ct"); // Set the dispatch table to something we'll recognize. test_ct.testDt.i = 0xaa55; // Verify that the list of all call types has one in it now. - EXPECT_EQ(all_types.size(), 1); + EXPECT_EQ(map.size(), 1); // Verify that that was our call type by verifying that the dispatch table // has our signature. - EXPECT_EQ(all_types[0]->getDispatch().i, 0xaa55); - - // Clear all_types, since call types don't clean up after themselves as - // they go away. - all_types.clear(); + EXPECT_EQ(map.begin()->second.getDispatch().i, 0xaa55); } TEST(CallTypeTest, DetectOne) { - auto &all_types = TestCallType::testGetAllTypes(); + auto &map = TestCallType::testGetMap(); // One option selected. - TestCallType option1; - option1.testAcceptArgs = 1; + TestCallType option1("option1"); option1.testIsDefault = true; option1.testDt.i = 1; - EXPECT_EQ(all_types.size(), 1); + EXPECT_EQ(map.size(), 1); - Args args; - args.i = 1; + Args args1({"--option1"}); EXPECT_FALSE(option1.testInitHappened); - auto &ct = CallType::detect(args); + auto *ct = CallType::detect(args1); // Verify that we selected the only option. EXPECT_TRUE(option1.testInitHappened); - EXPECT_EQ(ct.getDispatch().i, 1); + EXPECT_EQ(ct, &option1); // One option, selecting the default. option1.testInitHappened = false; // Args will not match. - args.i = 2; + Args args2({"--option2"}); - auto &def_ct = CallType::detect(args); + auto *def_ct = CallType::detect(args2); // Verify that the one option was defaulted to. EXPECT_TRUE(option1.testInitHappened); - EXPECT_EQ(def_ct.getDispatch().i, 1); - - // Reset all_types. - all_types.clear(); + EXPECT_EQ(def_ct, &option1); } TEST(CallTypeTest, DetectTwo) { - auto &all_types = TestCallType::testGetAllTypes(); + auto &map = TestCallType::testGetMap(); // One of two options selected. - TestCallType option1; - option1.testAcceptArgs = 1; + TestCallType option1("option1"); option1.testIsDefault = true; option1.testDt.i = 1; - TestCallType option2; - option2.testAcceptArgs = 2; + TestCallType option2("option2"); option2.testIsDefault = false; option2.testDt.i = 2; - EXPECT_EQ(all_types.size(), 2); + EXPECT_EQ(map.size(), 2); // Select the first option. - Args args; - args.i = 1; + Args args1({"--option1"}); EXPECT_FALSE(option1.testInitHappened); EXPECT_FALSE(option2.testInitHappened); - auto &ct1 = CallType::detect(args); + auto *ct1 = CallType::detect(args1); // Verify that we selected the first option. EXPECT_TRUE(option1.testInitHappened); EXPECT_FALSE(option2.testInitHappened); - EXPECT_EQ(ct1.getDispatch().i, 1); + EXPECT_EQ(ct1, &option1); option1.testInitHappened = false; option2.testInitHappened = false; // Select the second option. - args.i = 2; + Args args2({"--option2"}); - auto &ct2 = CallType::detect(args); + auto *ct2 = CallType::detect(args2); // Verify that we selected the second option. EXPECT_FALSE(option1.testInitHappened); EXPECT_TRUE(option2.testInitHappened); - EXPECT_EQ(ct2.getDispatch().i, 2); + EXPECT_EQ(ct2, &option2); option1.testInitHappened = false; option2.testInitHappened = false; // Default to the first option. - args.i = 3; + Args args3({"--option3"}); - auto &def_ct1 = CallType::detect(args); + auto *def_ct1 = CallType::detect(args3); // Verify that we selected the first option. EXPECT_TRUE(option1.testInitHappened); EXPECT_FALSE(option2.testInitHappened); - EXPECT_EQ(ct1.getDispatch().i, 1); + EXPECT_EQ(def_ct1, &option1); option1.testInitHappened = false; option2.testInitHappened = false; @@ -200,34 +186,31 @@ TEST(CallTypeTest, DetectTwo) option1.testIsDefault = false; option2.testIsDefault = true; - auto &def_ct2 = CallType::detect(args); + auto *def_ct2 = CallType::detect(args3); // Verify that we selected the second option. EXPECT_FALSE(option1.testInitHappened); EXPECT_TRUE(option2.testInitHappened); - EXPECT_EQ(ct2.getDispatch().i, 2); + EXPECT_EQ(def_ct2, &option2); option1.testInitHappened = false; option2.testInitHappened = false; - - // Reset all_types. - all_types.clear(); } TEST(CallTypeTest, Usage) { - auto &all_types = TestCallType::testGetAllTypes(); + auto &map = TestCallType::testGetMap(); - TestCallType ct1; + TestCallType ct1("ct1"); ct1.testBrief = "brief 1"; ct1.testDesc = "A longer description of call type 1, which is long."; - TestCallType ct2; + TestCallType ct2("ct2"); ct2.testBrief = "short 2"; ct2.testDesc = "Very verbose text saying what call type 2 is, " "and is different from 1."; - EXPECT_EQ(all_types.size(), 2); + EXPECT_EQ(map.size(), 2); auto summary = CallType::usageSummary(); @@ -241,7 +224,4 @@ TEST(CallTypeTest, Usage) EXPECT_THAT(summary, testing::HasSubstr(ct2.testBrief)); EXPECT_THAT(summary, testing::HasSubstr(ct2.testDesc)); EXPECT_THAT(summary, testing::HasSubstr(ct2.testDesc)); - - // Reset all_types. - all_types.clear(); } diff --git a/util/m5/src/call_type/addr.cc b/util/m5/src/call_type/addr.cc index ab26db73f..9a5fee3f2 100644 --- a/util/m5/src/call_type/addr.cc +++ b/util/m5/src/call_type/addr.cc @@ -59,16 +59,17 @@ constexpr uint64_t DefaultAddress = 0; class AddrCallType : public CallType { - private: public: + AddrCallType() : CallType("addr") {} + bool isDefault() const override { return CALL_TYPE_IS_DEFAULT; } const DispatchTable &getDispatch() const override { return addr_dispatch; } void printBrief(std::ostream &os) const override { - os << "--addr " << (DefaultAddrDefined ? "[address override]" : - "
"); + os << "--" << name << (DefaultAddrDefined ? " [address override]" : + "
"); } void @@ -81,15 +82,15 @@ class AddrCallType : public CallType } } - bool + CheckArgsResult checkArgs(Args &args) override { - static const std::string prefix = "--addr"; + const std::string prefix = "--" + name; uint64_t addr_override; // If the first argument doesn't start with --addr... if (!args.size() || args[0].substr(0, prefix.size()) != prefix) - return false; + return CheckArgsResult::NoMatch; const std::string &arg = args.pop().substr(prefix.size()); @@ -97,25 +98,25 @@ class AddrCallType : public CallType if (arg.size()) { // If it doesn't start with '=', it's malformed. if (arg[0] != '=') - usage(); + return CheckArgsResult::Usage; // Attempt to extract an address after the '='. if (!args.stoi(arg.substr(1), addr_override)) - usage(); + return CheckArgsResult::Usage; // If we found an address, use it to override m5op_addr. m5op_addr = addr_override; - return true; + return CheckArgsResult::Match; } // If an address override wasn't part of the first argument, check if // it's the second argument. If not, then there's no override. if (args.pop(addr_override)) { m5op_addr = addr_override; - return true; + return CheckArgsResult::Match; } // If the default address was not defined, an override is required. if (!DefaultAddrDefined) - usage(); + return CheckArgsResult::Usage; - return true; + return CheckArgsResult::Match; } void init() override { map_m5_mem(); } diff --git a/util/m5/src/call_type/inst.cc b/util/m5/src/call_type/inst.cc index a6af59550..97153f4fa 100644 --- a/util/m5/src/call_type/inst.cc +++ b/util/m5/src/call_type/inst.cc @@ -43,20 +43,11 @@ M5OP_FOREACH class InstCallType : public CallType { public: + InstCallType() : CallType("inst") {} + bool isDefault() const override { return CALL_TYPE_IS_DEFAULT; } const DispatchTable &getDispatch() const override { return inst_dispatch; } - bool - checkArgs(Args &args) override - { - if (args.size() && args[0] == "--inst") { - args.pop(); - return true; - } - return false; - } - - void printBrief(std::ostream &os) const override { os << "--inst"; } void printDesc(std::ostream &os) const override { diff --git a/util/m5/src/call_type/semi.cc b/util/m5/src/call_type/semi.cc index ddc192565..40a8e8d83 100644 --- a/util/m5/src/call_type/semi.cc +++ b/util/m5/src/call_type/semi.cc @@ -50,20 +50,11 @@ M5OP_FOREACH class SemiCallType : public CallType { public: + SemiCallType() : CallType("semi") {} + bool isDefault() const override { return CALL_TYPE_IS_DEFAULT; } const DispatchTable &getDispatch() const override { return semi_dispatch; } - bool - checkArgs(Args &args) override - { - if (args.size() && args[0] == "--semi") { - args.pop(); - return true; - } - return false; - } - - void printBrief(std::ostream &os) const override { os << "--semi"; } void printDesc(std::ostream &os) const override { diff --git a/util/m5/src/m5.cc b/util/m5/src/m5.cc index f8c1101f0..0e86817e1 100644 --- a/util/m5/src/m5.cc +++ b/util/m5/src/m5.cc @@ -53,9 +53,11 @@ main(int argc, const char *argv[]) if (!args.size()) usage(); - const DispatchTable &dt = CallType::detect(args).getDispatch(); + CallType *ct = CallType::detect(args); + if (!ct) + usage(); - if (!Command::run(dt, args)) + if (!Command::run(ct->getDispatch(), args)) usage(); exit(0); -- 2.30.2