Merge pull request ethereum#14243 from ethereum/do-not-optimize-yul-t…

…wice

Remove redundant Yul optimization from `IRGenerator`

Changelog.md

@@ -18,6 +18,7 @@ Bugfixes:
  * Commandline Interface: It is no longer possible to specify both ``--optimize-yul`` and ``--no-optimize-yul`` at the same time.
  * SMTChecker: Fix encoding of side-effects inside ``if`` and ``ternary conditional``statements in the BMC engine.
  * SMTChecker: Fix false negative when a verification target can be violated only by trusted external call from another public function.
+ * Yul Optimizer: Fix optimized IR being unnecessarily passed through the Yul optimizer again before bytecode generation.
 
 AST Changes:
  * AST: Add the ``experimentalSolidity`` field to the ``SourceUnit`` nodes, which indicate whether the experimental parsing mode has been enabled via ``pragma experimental solidity``.

libsolidity/codegen/ir/IRGenerationContext.h

@@ -23,7 +23,6 @@
 
 #include <libsolidity/ast/AST.h>
 #include <libsolidity/codegen/ir/IRVariable.h>
-#include <libsolidity/interface/OptimiserSettings.h>
 #include <libsolidity/interface/DebugSettings.h>
 
 #include <libsolidity/codegen/MultiUseYulFunctionCollector.h>
@@ -73,15 +72,13 @@ class IRGenerationContext
 		langutil::EVMVersion _evmVersion,
 		ExecutionContext _executionContext,
 		RevertStrings _revertStrings,
-		OptimiserSettings _optimiserSettings,
 		std::map<std::string, unsigned> _sourceIndices,
 		langutil::DebugInfoSelection const& _debugInfoSelection,
 		langutil::CharStreamProvider const* _soliditySourceProvider
 	):
 		m_evmVersion(_evmVersion),
 		m_executionContext(_executionContext),
 		m_revertStrings(_revertStrings),
-		m_optimiserSettings(std::move(_optimiserSettings)),
 		m_sourceIndices(std::move(_sourceIndices)),
 		m_debugInfoSelection(_debugInfoSelection),
 		m_soliditySourceProvider(_soliditySourceProvider)
@@ -176,7 +173,6 @@ class IRGenerationContext
 	langutil::EVMVersion m_evmVersion;
 	ExecutionContext m_executionContext;
 	RevertStrings m_revertStrings;
-	OptimiserSettings m_optimiserSettings;
 	std::map<std::string, unsigned> m_sourceIndices;
 	std::set<std::string> m_usedSourceNames;
 	ContractDefinition const* m_mostDerivedContract = nullptr;

libsolidity/codegen/ir/IRGenerator.cpp

@@ -89,36 +89,13 @@ set<CallableDeclaration const*, ASTNode::CompareByID> collectReachableCallables(
 
 }
 
-tuple<string, Json::Value, string, Json::Value> IRGenerator::run(
+string IRGenerator::run(
 	ContractDefinition const& _contract,
 	bytes const& _cborMetadata,
 	map<ContractDefinition const*, string_view const> const& _otherYulSources
 )
 {
-	string ir = yul::reindent(generate(_contract, _cborMetadata, _otherYulSources));
-
-	yul::YulStack asmStack(
-		m_evmVersion,
-		m_eofVersion,
-		yul::YulStack::Language::StrictAssembly,
-		m_optimiserSettings,
-		m_context.debugInfoSelection()
-	);
-	if (!asmStack.parseAndAnalyze("", ir))
-	{
-		string errorMessage;
-		for (auto const& error: asmStack.errors())
-			errorMessage += langutil::SourceReferenceFormatter::formatErrorInformation(
-				*error,
-				asmStack.charStream("")
-			);
-		solAssert(false, ir + "\n\nInvalid IR generated:\n" + errorMessage + "\n");
-	}
-	Json::Value irAst = asmStack.astJson();
-	asmStack.optimize();
-	Json::Value irOptAst = asmStack.astJson();
-
-	return {std::move(ir), std::move(irAst), asmStack.print(m_context.soliditySourceProvider()), std::move(irOptAst)};
+	return yul::reindent(generate(_contract, _cborMetadata, _otherYulSources));
 }
 
 string IRGenerator::generate(
@@ -1116,7 +1093,6 @@ void IRGenerator::resetContext(ContractDefinition const& _contract, ExecutionCon
 		m_evmVersion,
 		_context,
 		m_context.revertStrings(),
-		m_optimiserSettings,
 		m_context.sourceIndices(),
 		m_context.debugInfoSelection(),
 		m_context.soliditySourceProvider()

libsolidity/codegen/ir/IRGenerator.h

@@ -23,7 +23,6 @@
 
 #pragma once
 
-#include <libsolidity/interface/OptimiserSettings.h>
 #include <libsolidity/ast/ASTForward.h>
 #include <libsolidity/ast/CallGraph.h>
 #include <libsolidity/codegen/ir/IRGenerationContext.h>
@@ -50,29 +49,25 @@ class IRGenerator
 		langutil::EVMVersion _evmVersion,
 		std::optional<uint8_t> _eofVersion,
 		RevertStrings _revertStrings,
-		OptimiserSettings _optimiserSettings,
 		std::map<std::string, unsigned> _sourceIndices,
 		langutil::DebugInfoSelection const& _debugInfoSelection,
 		langutil::CharStreamProvider const* _soliditySourceProvider
 	):
 		m_evmVersion(_evmVersion),
 		m_eofVersion(_eofVersion),
-		m_optimiserSettings(_optimiserSettings),
 		m_context(
 			_evmVersion,
 			ExecutionContext::Creation,
 			_revertStrings,
-			std::move(_optimiserSettings),
 			std::move(_sourceIndices),
 			_debugInfoSelection,
 			_soliditySourceProvider
 		),
 		m_utils(_evmVersion, m_context.revertStrings(), m_context.functionCollector())
 	{}
 
-	/// Generates and returns the IR code, in unoptimized and optimized form
-	/// (or just pretty-printed, depending on the optimizer settings).
-	std::tuple<std::string, Json::Value, std::string, Json::Value> run(
+	/// Generates and returns (unoptimized) IR code.
+	std::string run(
 		ContractDefinition const& _contract,
 		bytes const& _cborMetadata,
 		std::map<ContractDefinition const*, std::string_view const> const& _otherYulSources
@@ -143,7 +138,6 @@ class IRGenerator
 
 	langutil::EVMVersion const m_evmVersion;
 	std::optional<uint8_t> const m_eofVersion;
-	OptimiserSettings const m_optimiserSettings;
 
 	IRGenerationContext m_context;
 	YulUtilFunctions m_utils;

libsolidity/interface/CompilerStack.cpp

@@ -70,6 +70,7 @@
 
 #include <liblangutil/Scanner.h>
 #include <liblangutil/SemVerHandler.h>
+#include <liblangutil/SourceReferenceFormatter.h>
 
 #include <libevmasm/Exceptions.h>
 
@@ -1468,22 +1469,38 @@ void CompilerStack::generateIR(ContractDefinition const& _contract)
 		m_evmVersion,
 		m_eofVersion,
 		m_revertStrings,
-		m_optimiserSettings,
 		sourceIndices(),
 		m_debugInfoSelection,
 		this
 	);
-
-	tie(
-		compiledContract.yulIR,
-		compiledContract.yulIRAst,
-		compiledContract.yulIROptimized,
-		compiledContract.yulIROptimizedAst
-	) = generator.run(
+	compiledContract.yulIR = generator.run(
 		_contract,
 		createCBORMetadata(compiledContract, /* _forIR */ true),
 		otherYulSources
 	);
+
+	yul::YulStack stack(
+		m_evmVersion,
+		m_eofVersion,
+		yul::YulStack::Language::StrictAssembly,
+		m_optimiserSettings,
+		m_debugInfoSelection
+	);
+	if (!stack.parseAndAnalyze("", compiledContract.yulIR))
+	{
+		string errorMessage;
+		for (auto const& error: stack.errors())
+			errorMessage += langutil::SourceReferenceFormatter::formatErrorInformation(
+				*error,
+				stack.charStream("")
+			);
+		solAssert(false, compiledContract.yulIR + "\n\nInvalid IR generated:\n" + errorMessage + "\n");
+	}
+
+	compiledContract.yulIRAst = stack.astJson();
+	stack.optimize();
+	compiledContract.yulIROptimized = stack.print(this);
+	compiledContract.yulIROptimizedAst = stack.astJson();
 }
 
 void CompilerStack::generateEVMFromIR(ContractDefinition const& _contract)
@@ -1508,8 +1525,8 @@ void CompilerStack::generateEVMFromIR(ContractDefinition const& _contract)
 		m_optimiserSettings,
 		m_debugInfoSelection
 	);
-	stack.parseAndAnalyze("", compiledContract.yulIROptimized);
-	stack.optimize();
+	bool analysisSuccessful = stack.parseAndAnalyze("", compiledContract.yulIROptimized);
+	solAssert(analysisSuccessful);
 
 	//cout << yul::AsmPrinter{}(*stack.parserResult()->code) << endl;
 

test/cmdlineTests/~via_ir_equivalence/test.sh

@@ -33,7 +33,8 @@ function test_via_ir_equivalence()
     for yul_file in $(find . -name "${output_file_prefix}*.yul" | sort -V); do
         asm_output_two_stage+=$(
             msg_on_error --no-stderr \
-                "$SOLC" --strict-assembly --asm "${optimizer_flags[@]}" "$yul_file" | stripCLIDecorations
+                "$SOLC" --strict-assembly --asm "${optimizer_flags[@]}" --no-optimize-yul "$yul_file" |
+                    stripCLIDecorations
         )
     done
 
@@ -50,7 +51,8 @@ function test_via_ir_equivalence()
     for yul_file in $(find . -name "${output_file_prefix}*.yul" | sort -V); do
         bin_output_two_stage+=$(
             msg_on_error --no-stderr \
-                "$SOLC" --strict-assembly --bin "${optimizer_flags[@]}" "$yul_file" | stripCLIDecorations
+                "$SOLC" --strict-assembly --bin "${optimizer_flags[@]}" "$yul_file" --no-optimize-yul |
+                    stripCLIDecorations
         )
     done