diff --git a/Changelog.md b/Changelog.md index 9b1f3fe86..32d3db1d4 100644 --- a/Changelog.md +++ b/Changelog.md @@ -15,6 +15,8 @@ Compiler Features: Bugfixes: * Code generator: Do not pad empty string literals with a single 32-byte zero field in the ABI coder v1. * SMTChecker: Fix internal compiler error when doing bitwise compound assignment with string literals. + * Yul Optimizer: Fix a bug in NameSimplifier where a new name created by NameSimplifier could also be created by NameDispenser. + * Yul Optimizer: Removed NameSimplifier from optimization steps available to users. ### 0.7.5 (2020-11-18) diff --git a/libsolidity/interface/OptimiserSettings.h b/libsolidity/interface/OptimiserSettings.h index 1a2095198..4b1cd7c4e 100644 --- a/libsolidity/interface/OptimiserSettings.h +++ b/libsolidity/interface/OptimiserSettings.h @@ -32,7 +32,7 @@ namespace solidity::frontend struct OptimiserSettings { static char constexpr DefaultYulOptimiserSteps[] = - "NdhfoDgvulfnTUtnIf" // None of these can make stack problems worse + "dhfoDgvulfnTUtnIf" // None of these can make stack problems worse "[" "xarrscLM" // Turn into SSA and simplify "cCTUtTOntnfDIul" // Perform structural simplification @@ -47,7 +47,7 @@ struct OptimiserSettings "gvif" // Run full inliner "CTUcarrLsTOtfDncarrIulc" // SSA plus simplify "]" - "jmuljuljul VcTOcul jmulN"; // Make source short and pretty + "jmuljuljul VcTOcul jmul"; // Make source short and pretty /// No optimisations at all - not recommended. static OptimiserSettings none() diff --git a/libyul/optimiser/NameDispenser.cpp b/libyul/optimiser/NameDispenser.cpp index 7c9fcc6bc..01d33919f 100644 --- a/libyul/optimiser/NameDispenser.cpp +++ b/libyul/optimiser/NameDispenser.cpp @@ -35,8 +35,9 @@ using namespace solidity::yul; using namespace solidity::util; NameDispenser::NameDispenser(Dialect const& _dialect, Block const& _ast, set _reservedNames): - NameDispenser(_dialect, NameCollector(_ast).names() + std::move(_reservedNames)) + NameDispenser(_dialect, NameCollector(_ast).names() + _reservedNames) { + m_reservedNames = move(_reservedNames); } NameDispenser::NameDispenser(Dialect const& _dialect, set _usedNames): @@ -61,3 +62,9 @@ bool NameDispenser::illegalName(YulString _name) { return isRestrictedIdentifier(m_dialect, _name) || m_usedNames.count(_name); } + +void NameDispenser::reset(Block const& _ast) +{ + m_usedNames = NameCollector(_ast).names() + m_reservedNames; + m_counter = 0; +} diff --git a/libyul/optimiser/NameDispenser.h b/libyul/optimiser/NameDispenser.h index e379708d0..6e655e416 100644 --- a/libyul/optimiser/NameDispenser.h +++ b/libyul/optimiser/NameDispenser.h @@ -51,11 +51,19 @@ public: /// return it. void markUsed(YulString _name) { m_usedNames.insert(_name); } -private: + std::set const& usedNames() { return m_usedNames; } + + /// Returns true if `_name` is either used or is a restricted identifier. bool illegalName(YulString _name); + /// Resets `m_usedNames` with *only* the names that are used in the AST. Also resets value of + /// `m_counter` to zero. + void reset(Block const& _ast); + +private: Dialect const& m_dialect; std::set m_usedNames; + std::set m_reservedNames; size_t m_counter = 0; }; diff --git a/libyul/optimiser/NameSimplifier.cpp b/libyul/optimiser/NameSimplifier.cpp index 741280edd..81379297d 100644 --- a/libyul/optimiser/NameSimplifier.cpp +++ b/libyul/optimiser/NameSimplifier.cpp @@ -19,6 +19,8 @@ #include #include #include +#include +#include #include #include @@ -28,19 +30,13 @@ using namespace solidity::yul; using namespace std; -NameSimplifier::NameSimplifier( - OptimiserStepContext& _context, - Block const& _ast -): - m_context(_context), - m_usedNames(_context.reservedIdentifiers) +NameSimplifier::NameSimplifier(OptimiserStepContext& _context, Block const& _ast): + m_context(_context) { - for (YulString name: m_usedNames) + for (YulString name: _context.reservedIdentifiers) m_translations[name] = name; - set allNames = NameCollector(_ast).names(); - m_usedNames += allNames; - for (YulString name: allNames) + for (YulString const& name: NameCollector(_ast).names()) findSimplification(name); } @@ -77,7 +73,7 @@ void NameSimplifier::operator()(FunctionCall& _funCall) ASTModifier::operator()(_funCall); } -void NameSimplifier::findSimplification(YulString _name) +void NameSimplifier::findSimplification(YulString const& _name) { if (m_translations.count(_name)) return; @@ -98,19 +94,19 @@ void NameSimplifier::findSimplification(YulString _name) {regex("index_access_t_array"), "index_access"}, {regex("[0-9]*_$"), ""} }; + for (auto const& [pattern, substitute]: replacements) { string candidate = regex_replace(name, pattern, substitute); - if ( - !isRestrictedIdentifier(m_context.dialect, YulString(candidate)) && - !m_usedNames.count(YulString(candidate)) - ) + if (!m_context.dispenser.illegalName(YulString(candidate))) name = candidate; } + if (name != _name.str()) { - m_usedNames.insert(YulString(name)); - m_translations[_name] = YulString(name); + YulString newName{name}; + m_context.dispenser.markUsed(newName); + m_translations[_name] = move(newName); } } diff --git a/libyul/optimiser/NameSimplifier.h b/libyul/optimiser/NameSimplifier.h index 0edbc595f..34cf69451 100644 --- a/libyul/optimiser/NameSimplifier.h +++ b/libyul/optimiser/NameSimplifier.h @@ -57,19 +57,15 @@ public: void operator()(FunctionDefinition& _funDef) override; private: - NameSimplifier( - OptimiserStepContext& _context, - Block const& _ast - ); + NameSimplifier(OptimiserStepContext& _context, Block const& _ast); /// Tries to rename a list of variables. void renameVariables(std::vector& _variables); - void findSimplification(YulString _name); + void findSimplification(YulString const& _name); void translate(YulString& _name); OptimiserStepContext& m_context; - std::set m_usedNames; std::map m_translations; }; diff --git a/libyul/optimiser/Suite.cpp b/libyul/optimiser/Suite.cpp index 8dcc45616..3f59e89e6 100644 --- a/libyul/optimiser/Suite.cpp +++ b/libyul/optimiser/Suite.cpp @@ -105,6 +105,7 @@ void OptimiserSuite::run( // ForLoopInitRewriter. Run them first to be able to run arbitrary sequences safely. suite.runSequence("hfgo", ast); + NameSimplifier::run(suite.m_context, ast); // Now the user-supplied part suite.runSequence(_optimisationSequence, ast); @@ -140,6 +141,9 @@ void OptimiserSuite::run( if (ast.statements.size() > 1 && std::get(ast.statements.front()).statements.empty()) ast.statements.erase(ast.statements.begin()); } + + suite.m_dispenser.reset(ast); + NameSimplifier::run(suite.m_context, ast); VarNameCleaner::run(suite.m_context, ast); *_object.analysisInfo = AsmAnalyzer::analyzeStrictAssertCorrect(_dialect, _object); @@ -191,7 +195,6 @@ map> const& OptimiserSuite::allSteps() LiteralRematerialiser, LoadResolver, LoopInvariantCodeMotion, - NameSimplifier, RedundantAssignEliminator, ReasoningBasedSimplifier, Rematerialiser, @@ -203,6 +206,7 @@ map> const& OptimiserSuite::allSteps() VarDeclInitializer >(); // Does not include VarNameCleaner because it destroys the property of unique names. + // Does not include NameSimplifier. return instance; } @@ -230,7 +234,6 @@ map const& OptimiserSuite::stepNameToAbbreviationMap() {LiteralRematerialiser::name, 'T'}, {LoadResolver::name, 'L'}, {LoopInvariantCodeMotion::name, 'M'}, - {NameSimplifier::name, 'N'}, {ReasoningBasedSimplifier::name, 'R'}, {RedundantAssignEliminator::name, 'r'}, {Rematerialiser::name, 'm'}, diff --git a/test/yulPhaser/Chromosome.cpp b/test/yulPhaser/Chromosome.cpp index 1f66398be..b42086202 100644 --- a/test/yulPhaser/Chromosome.cpp +++ b/test/yulPhaser/Chromosome.cpp @@ -138,7 +138,7 @@ BOOST_AUTO_TEST_CASE(output_operator_should_create_concise_and_unambiguous_strin BOOST_TEST(chromosome.length() == allSteps.size()); BOOST_TEST(chromosome.optimisationSteps() == allSteps); - BOOST_TEST(toString(chromosome) == "flcCUnDvejsxIOoighTLMNRrmVatpud"); + BOOST_TEST(toString(chromosome) == "flcCUnDvejsxIOoighTLMRrmVatpud"); } BOOST_AUTO_TEST_CASE(optimisationSteps_should_translate_chromosomes_genes_to_optimisation_step_names)