From 9c8187bd29170678675b1d6482e012116202add7 Mon Sep 17 00:00:00 2001 From: cameel Date: Wed, 29 Jan 2020 17:37:10 +0100 Subject: [PATCH 1/5] CommonData: Add invertMap() function --- libsolutil/CommonData.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/libsolutil/CommonData.h b/libsolutil/CommonData.h index c7c9d80e3..45f19bcb5 100644 --- a/libsolutil/CommonData.h +++ b/libsolutil/CommonData.h @@ -156,6 +156,23 @@ T convertContainer(U&& _from) }; } +/// Gets a @a K -> @a V map and returns a map where values from the original map are keys and keys +/// from the original map are values. +/// +/// @pre @a originalMap must have unique values. +template +std::map invertMap(std::map const& originalMap) +{ + std::map inverseMap; + for (auto const& [key, value]: originalMap) + { + assert(inverseMap.count(value) == 0); + inverseMap.insert({value, key}); + } + + return inverseMap; +} + // String conversion functions, mainly to/from hex/nibble/byte representations. enum class WhenError From 4e7c1c7876dadd3b406b9f0539cf8dd8bc8ece52 Mon Sep 17 00:00:00 2001 From: cameel Date: Fri, 24 Jan 2020 22:03:44 +0100 Subject: [PATCH 2/5] OptimiserSuite: Add two maps for converting between step names and abbreviations - Abbreviations match those used in yulopti. - I considered using boost::bimap but I think it would be an overkill. Two simple maps are good enough in a situation where data is constant and there isn't much of it. - I could also use InvertibleMap from libsolutil but I don't even need any of its methods since my map is a constant. I also don't need the inverted map to store sets because my values are unique. - The reverseMap() is generic enough to be moved to some global file with utilities but I don't sure if it's going to actually be useful to others in practice. --- libyul/optimiser/Suite.cpp | 43 ++++++++++++++++++++++++++++++++++++++ libyul/optimiser/Suite.h | 2 ++ 2 files changed, 45 insertions(+) diff --git a/libyul/optimiser/Suite.cpp b/libyul/optimiser/Suite.cpp index b57cf7645..47c245752 100644 --- a/libyul/optimiser/Suite.cpp +++ b/libyul/optimiser/Suite.cpp @@ -370,6 +370,49 @@ map> const& OptimiserSuite::allSteps() return instance; } +map const& OptimiserSuite::stepNameToAbbreviationMap() +{ + static map lookupTable{ + {BlockFlattener::name, 'f'}, + {CommonSubexpressionEliminator::name, 'c'}, + {ConditionalSimplifier::name, 'C'}, + {ConditionalUnsimplifier::name, 'U'}, + {ControlFlowSimplifier::name, 'n'}, + {DeadCodeEliminator::name, 'D'}, + {EquivalentFunctionCombiner::name, 'v'}, + {ExpressionInliner::name, 'e'}, + {ExpressionJoiner::name, 'j'}, + {ExpressionSimplifier::name, 's'}, + {ExpressionSplitter::name, 'x'}, + {ForLoopConditionIntoBody::name, 'I'}, + {ForLoopConditionOutOfBody::name, 'O'}, + {ForLoopInitRewriter::name, 'o'}, + {FullInliner::name, 'i'}, + {FunctionGrouper::name, 'g'}, + {FunctionHoister::name, 'h'}, + {LiteralRematerialiser::name, 'T'}, + {LoadResolver::name, 'L'}, + {LoopInvariantCodeMotion::name, 'M'}, + {RedundantAssignEliminator::name, 'r'}, + {Rematerialiser::name, 'm'}, + {SSAReverser::name, 'V'}, + {SSATransform::name, 'a'}, + {StructuralSimplifier::name, 't'}, + {UnusedPruner::name, 'u'}, + {VarDeclInitializer::name, 'd'}, + }; + yulAssert(lookupTable.size() == allSteps().size(), ""); + + return lookupTable; +} + +map const& OptimiserSuite::stepAbbreviationToNameMap() +{ + static map lookupTable = util::invertMap(stepNameToAbbreviationMap()); + + return lookupTable; +} + void OptimiserSuite::runSequence(std::vector const& _steps, Block& _ast) { unique_ptr copy; diff --git a/libyul/optimiser/Suite.h b/libyul/optimiser/Suite.h index 299a465fb..6ce6b485c 100644 --- a/libyul/optimiser/Suite.h +++ b/libyul/optimiser/Suite.h @@ -62,6 +62,8 @@ public: void runSequence(std::vector const& _steps, Block& _ast); static std::map> const& allSteps(); + static std::map const& stepNameToAbbreviationMap(); + static std::map const& stepAbbreviationToNameMap(); private: OptimiserSuite( From 4129c2749597419fe31adc46e386c9d0248696f9 Mon Sep 17 00:00:00 2001 From: cameel Date: Fri, 24 Jan 2020 23:51:35 +0100 Subject: [PATCH 3/5] [yulopti] Replace hard-coded step list with OptimiserSuite's maps --- test/tools/yulopti.cpp | 126 +++++------------------------------------ 1 file changed, 14 insertions(+), 112 deletions(-) diff --git a/test/tools/yulopti.cpp b/test/tools/yulopti.cpp index b0c0e0b35..fa291672d 100644 --- a/test/tools/yulopti.cpp +++ b/test/tools/yulopti.cpp @@ -30,40 +30,11 @@ #include #include -#include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include #include -#include -#include -#include #include -#include -#include -#include #include -#include -#include +#include #include @@ -71,6 +42,7 @@ #include +#include #include #include #include @@ -150,90 +122,26 @@ public: cout << ' ' << char(option) << endl; OptimiserStepContext context{m_dialect, *m_nameDispenser, reservedIdentifiers}; - switch (option) + + map const& abbreviationMap = OptimiserSuite::stepAbbreviationToNameMap(); + assert(abbreviationMap.count('q') == 0 && "We have chosen 'q' for quit"); + assert(abbreviationMap.count('p') == 0 && "We have chosen 'p' for StackCompressor"); + + auto abbreviationAndName = abbreviationMap.find(option); + if (abbreviationAndName != abbreviationMap.end()) + { + OptimiserStep const& step = *OptimiserSuite::allSteps().at(abbreviationAndName->second); + step.run(context, *m_ast); + } + else switch (option) { case 'q': return; - case 'f': - BlockFlattener::run(context, *m_ast); - break; - case 'o': - ForLoopInitRewriter::run(context, *m_ast); - break; - case 'O': - ForLoopConditionOutOfBody::run(context, *m_ast); - break; - case 'I': - ForLoopConditionIntoBody::run(context, *m_ast); - break; - case 'c': - CommonSubexpressionEliminator::run(context, *m_ast); - break; - case 'C': - ConditionalSimplifier::run(context, *m_ast); - break; - case 'U': - ConditionalUnsimplifier::run(context, *m_ast); - break; - case 'd': - VarDeclInitializer::run(context, *m_ast); - break; case 'l': VarNameCleaner::run(context, *m_ast); // VarNameCleaner destroys the unique names guarantee of the disambiguator. disambiguated = false; break; - case 'x': - ExpressionSplitter::run(context, *m_ast); - break; - case 'j': - ExpressionJoiner::run(context, *m_ast); - break; - case 'g': - FunctionGrouper::run(context, *m_ast); - break; - case 'h': - FunctionHoister::run(context, *m_ast); - break; - case 'e': - ExpressionInliner::run(context, *m_ast); - break; - case 'i': - FullInliner::run(context, *m_ast); - break; - case 's': - ExpressionSimplifier::run(context, *m_ast); - break; - case 't': - StructuralSimplifier::run(context, *m_ast); - break; - case 'T': - LiteralRematerialiser::run(context, *m_ast); - break; - case 'n': - ControlFlowSimplifier::run(context, *m_ast); - break; - case 'u': - UnusedPruner::run(context, *m_ast); - break; - case 'D': - DeadCodeEliminator::run(context, *m_ast); - break; - case 'a': - SSATransform::run(context, *m_ast); - break; - case 'r': - RedundantAssignEliminator::run(context, *m_ast); - break; - case 'm': - Rematerialiser::run(context, *m_ast); - break; - case 'v': - EquivalentFunctionCombiner::run(context, *m_ast); - break; - case 'V': - SSAReverser::run(context, *m_ast); - break; case 'p': { Object obj; @@ -241,12 +149,6 @@ public: StackCompressor::run(m_dialect, obj, true, 16); break; } - case 'L': - LoadResolver::run(context, *m_ast); - break; - case 'M': - LoopInvariantCodeMotion::run(context, *m_ast); - break; default: cout << "Unknown option." << endl; } From 5fbc4d4afaafe56f9201a8589ea0fc2a5f61d6dc Mon Sep 17 00:00:00 2001 From: cameel Date: Sat, 25 Jan 2020 00:46:16 +0100 Subject: [PATCH 4/5] [yulopti] Automate printing of the usage banner - This now displays internal step names rather than human-readable ones but the internal ones are readable enough and it's not something worth creating another map. - Options in the banner are now aligned in columns and thus easier to read. --- test/tools/yulopti.cpp | 54 +++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/test/tools/yulopti.cpp b/test/tools/yulopti.cpp index fa291672d..de074e642 100644 --- a/test/tools/yulopti.cpp +++ b/test/tools/yulopti.cpp @@ -42,7 +42,6 @@ #include -#include #include #include #include @@ -94,6 +93,40 @@ public: return true; } + void printUsageBanner( + map const& _optimizationSteps, + map const& _extraOptions, + size_t _columns + ) + { + auto hasShorterString = [](auto const& a, auto const& b){ return a.second.size() < b.second.size(); }; + size_t longestDescriptionLength = max( + max_element(_optimizationSteps.begin(), _optimizationSteps.end(), hasShorterString)->second.size(), + max_element(_extraOptions.begin(), _extraOptions.end(), hasShorterString)->second.size() + ); + + size_t index = 0; + auto printPair = [&](auto const& optionAndDescription) + { + cout << optionAndDescription.first << ": "; + cout << setw(longestDescriptionLength) << setiosflags(ios::left); + cout << optionAndDescription.second << " "; + + ++index; + if (index % _columns == 0) + cout << endl; + }; + + for (auto const& optionAndDescription: _extraOptions) + { + yulAssert(_optimizationSteps.count(optionAndDescription.first) == 0, ""); + printPair(optionAndDescription); + } + + for (auto const& abbreviationAndName: _optimizationSteps) + printPair(abbreviationAndName); + } + void runInteractive(string source) { bool disambiguated = false; @@ -111,22 +144,21 @@ public: m_nameDispenser = make_shared(m_dialect, *m_ast, reservedIdentifiers); disambiguated = true; } - cout << "(q)uit/(f)latten/(c)se/initialize var(d)ecls/(x)plit/(j)oin/(g)rouper/(h)oister/" << endl; - cout << " (e)xpr inline/(i)nline/(s)implify/varname c(l)eaner/(u)nusedprune/ss(a) transform/" << endl; - cout << " (r)edundant assign elim./re(m)aterializer/f(o)r-loop-init-rewriter/for-loop-condition-(I)nto-body/" << endl; - cout << " for-loop-condition-(O)ut-of-body/s(t)ructural simplifier/equi(v)alent function combiner/ssa re(V)erser/" << endl; - cout << " co(n)trol flow simplifier/stack com(p)ressor/(D)ead code eliminator/(L)oad resolver/" << endl; - cout << " (C)onditional simplifier/conditional (U)nsimplifier/loop-invariant code (M)otion?" << endl; + map const& abbreviationMap = OptimiserSuite::stepAbbreviationToNameMap(); + map const& extraOptions = { + {'q', "quit"}, + {'l', "VarNameCleaner"}, + {'p', "StackCompressor"}, + }; + + printUsageBanner(abbreviationMap, extraOptions, 4); + cout << "? "; cout.flush(); int option = readStandardInputChar(); cout << ' ' << char(option) << endl; OptimiserStepContext context{m_dialect, *m_nameDispenser, reservedIdentifiers}; - map const& abbreviationMap = OptimiserSuite::stepAbbreviationToNameMap(); - assert(abbreviationMap.count('q') == 0 && "We have chosen 'q' for quit"); - assert(abbreviationMap.count('p') == 0 && "We have chosen 'p' for StackCompressor"); - auto abbreviationAndName = abbreviationMap.find(option); if (abbreviationAndName != abbreviationMap.end()) { From c4f8df327270727e65b8d58516ea3d73878df5e1 Mon Sep 17 00:00:00 2001 From: cameel Date: Thu, 30 Jan 2020 02:12:54 +0100 Subject: [PATCH 5/5] Workaround for clang 5.0.0 on Ubuntu Trusty in Travis CI failing to compile a structural binding clang fails with: /home/travis/build/ethereum/solidity/libsolutil/CommonData.h:167:19: error: unused variable '' [-Werror,-Wunused-variable] for (auto const& [key, value]: originalMap) --- libsolutil/CommonData.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libsolutil/CommonData.h b/libsolutil/CommonData.h index 45f19bcb5..5afb65470 100644 --- a/libsolutil/CommonData.h +++ b/libsolutil/CommonData.h @@ -164,10 +164,10 @@ template std::map invertMap(std::map const& originalMap) { std::map inverseMap; - for (auto const& [key, value]: originalMap) + for (auto const& originalPair: originalMap) { - assert(inverseMap.count(value) == 0); - inverseMap.insert({value, key}); + assert(inverseMap.count(originalPair.second) == 0); + inverseMap.insert({originalPair.second, originalPair.first}); } return inverseMap;