Merge pull request #13378 from ethereum/forLoopSpecialCaseCodegen

unchecked increment for simple counter for loops

Author: ekpyron

Changelog.md

@@ -5,6 +5,7 @@ Language Features:
 
 
 Compiler Features:
+ * Code Generator: Remove redundant overflow checks of certain ``for`` loops when the counter variable cannot overflow.
  * Commandline Interface: Add ``--no-import-callback`` option that prevents the compiler from loading source files not given explicitly on the CLI or in Standard JSON input.
  * Commandline Interface: Use proper severity and coloring also for error messages produced outside of the compilation pipeline.
  * EVM: Deprecate support for "homestead", "tangerineWhistle", "spuriousDragon" and "byzantium" EVM versions.

docs/using-the-compiler.rst

@@ -287,6 +287,9 @@ Input Description
             "cse": false,
             // Optimize representation of literal numbers and strings in code.
             "constantOptimizer": false,
+            // Use unchecked arithmetic when incrementing the counter of for loops
+            // under certain circumstances. It is always on if no details are given.
+            "simpleCounterForLoopUncheckedIncrement": true,
             // The new Yul optimizer. Mostly operates on the code of ABI coder v2
             // and inline assembly.
             // It is activated together with the global optimizer setting

libsolidity/analysis/PostTypeChecker.cpp

@@ -24,6 +24,10 @@
 #include <liblangutil/SemVerHandler.h>
 #include <libsolutil/Algorithms.h>
 #include <libsolutil/FunctionSelector.h>
+#include <libyul/optimiser/ASTWalker.h>
+#include <libyul/AST.h>
+
+#include <range/v3/algorithm/any_of.hpp>
 
 #include <memory>
 
@@ -129,6 +133,17 @@ void PostTypeChecker::endVisit(ModifierInvocation const& _modifierInvocation)
 	callEndVisit(_modifierInvocation);
 }
 
+
+bool PostTypeChecker::visit(ForStatement const& _forStatement)
+{
+	return callVisit(_forStatement);
+}
+
+void PostTypeChecker::endVisit(ForStatement const& _forStatement)
+{
+	callEndVisit(_forStatement);
+}
+
 namespace
 {
 struct ConstStateVarCircularReferenceChecker: public PostTypeChecker::Checker
@@ -421,6 +436,116 @@ struct ReservedErrorSelector: public PostTypeChecker::Checker
 	}
 };
 
+class YulLValueChecker : public solidity::yul::ASTWalker
+{
+public:
+	YulLValueChecker(ASTString const& _identifierName): m_identifierName(_identifierName) {}
+	bool willBeWrittenTo() const { return m_willBeWrittenTo; }
+	using solidity::yul::ASTWalker::operator();
+	void operator()(solidity::yul::Assignment const& _assignment) override
+	{
+		if (m_willBeWrittenTo)
+			return;
+
+		if (ranges::any_of(
+			_assignment.variableNames,
+			[&](auto const& yulIdentifier) { return yulIdentifier.name.str() == m_identifierName; }
+		))
+			m_willBeWrittenTo = true;
+	}
+private:
+	ASTString const& m_identifierName;
+	bool m_willBeWrittenTo = false;
+};
+
+class LValueChecker: public ASTConstVisitor
+{
+public:
+	LValueChecker(Identifier const& _identifier):
+		m_declaration(_identifier.annotation().referencedDeclaration)
+	{}
+	bool willBeWrittenTo() const { return m_willBeWrittenTo; }
+	void endVisit(Identifier const& _identifier) override
+	{
+		if (m_willBeWrittenTo)
+			return;
+
+		solAssert(_identifier.annotation().referencedDeclaration);
+		if (
+			*_identifier.annotation().referencedDeclaration == *m_declaration &&
+			_identifier.annotation().willBeWrittenTo
+		)
+			m_willBeWrittenTo = true;
+	}
+	void endVisit(InlineAssembly const& _inlineAssembly) override
+	{
+		if (m_willBeWrittenTo)
+			return;
+
+		YulLValueChecker yulChecker{m_declaration->name()};
+		yulChecker(_inlineAssembly.operations());
+		m_willBeWrittenTo = yulChecker.willBeWrittenTo();
+	}
+private:
+	Declaration const* m_declaration{};
+	bool m_willBeWrittenTo = false;
+};
+
+struct SimpleCounterForLoopChecker: public PostTypeChecker::Checker
+{
+	SimpleCounterForLoopChecker(ErrorReporter& _errorReporter): Checker(_errorReporter) {}
+	bool visit(ForStatement const& _forStatement) override
+	{
+		_forStatement.annotation().isSimpleCounterLoop = isSimpleCounterLoop(_forStatement);
+		return true;
+	}
+	bool isSimpleCounterLoop(ForStatement const& _forStatement) const
+	{
+		auto const* simpleCondition = dynamic_cast<BinaryOperation const*>(_forStatement.condition());
+		if (!simpleCondition || simpleCondition->getOperator() != Token::LessThan || simpleCondition->userDefinedFunctionType())
+			return false;
+		if (!_forStatement.loopExpression())
+			return false;
+
+		auto const* simplePostExpression = dynamic_cast<UnaryOperation const*>(&_forStatement.loopExpression()->expression());
+		// This matches both operators ++i and i++
+		if (!simplePostExpression || simplePostExpression->getOperator() != Token::Inc || simplePostExpression->userDefinedFunctionType())
+			return false;
+
+		auto const* lhsIdentifier = dynamic_cast<Identifier const*>(&simpleCondition->leftExpression());
+		auto const* lhsIntegerType = dynamic_cast<IntegerType const*>(simpleCondition->leftExpression().annotation().type);
+		auto const* commonIntegerType = dynamic_cast<IntegerType const*>(simpleCondition->annotation().commonType);
+
+		if (!lhsIdentifier || !lhsIntegerType || !commonIntegerType || *lhsIntegerType != *commonIntegerType)
+			return false;
+
+		auto const* incExpressionIdentifier = dynamic_cast<Identifier const*>(&simplePostExpression->subExpression());
+		if (
+			!incExpressionIdentifier ||
+			incExpressionIdentifier->annotation().referencedDeclaration != lhsIdentifier->annotation().referencedDeclaration
+		)
+			return false;
+
+		solAssert(incExpressionIdentifier->annotation().referencedDeclaration);
+		if (
+			auto const* incVariableDeclaration = dynamic_cast<VariableDeclaration const*>(
+				incExpressionIdentifier->annotation().referencedDeclaration
+			);
+			incVariableDeclaration &&
+			!incVariableDeclaration->isLocalVariable()
+		)
+			return false;
+
+		solAssert(lhsIdentifier);
+		LValueChecker lValueChecker{*lhsIdentifier};
+		simpleCondition->rightExpression().accept(lValueChecker);
+		if (!lValueChecker.willBeWrittenTo())
+			_forStatement.body().accept(lValueChecker);
+
+		return !lValueChecker.willBeWrittenTo();
+	}
+};
+
 }
 
 
@@ -432,4 +557,5 @@ PostTypeChecker::PostTypeChecker(langutil::ErrorReporter& _errorReporter): m_err
 	m_checkers.push_back(std::make_shared<EventOutsideEmitErrorOutsideRevertChecker>(_errorReporter));
 	m_checkers.push_back(std::make_shared<NoVariablesInInterfaceChecker>(_errorReporter));
 	m_checkers.push_back(std::make_shared<ReservedErrorSelector>(_errorReporter));
+	m_checkers.push_back(std::make_shared<SimpleCounterForLoopChecker>(_errorReporter));
 }

libsolidity/analysis/PostTypeChecker.h

@@ -97,6 +97,9 @@ class PostTypeChecker: private ASTConstVisitor
 	bool visit(ModifierInvocation const& _modifierInvocation) override;
 	void endVisit(ModifierInvocation const& _modifierInvocation) override;
 
+	bool visit(ForStatement const& _forStatement) override;
+	void endVisit(ForStatement const& _forStatement) override;
+
 	template <class T>
 	bool callVisit(T const& _node)
 	{

libsolidity/ast/ASTAnnotations.h

@@ -238,6 +238,7 @@ struct TryCatchClauseAnnotation: ASTAnnotation, ScopableAnnotation
 
 struct ForStatementAnnotation: StatementAnnotation, ScopableAnnotation
 {
+	util::SetOnce<bool> isSimpleCounterLoop;
 };
 
 struct ReturnAnnotation: StatementAnnotation

libsolidity/ast/ASTJsonExporter.cpp

@@ -742,12 +742,18 @@ bool ASTJsonExporter::visit(WhileStatement const& _node)
 
 bool ASTJsonExporter::visit(ForStatement const& _node)
 {
-	setJsonNode(_node, "ForStatement", {
+
+	std::vector<std::pair<std::string, Json::Value>> attributes = {
 		std::make_pair("initializationExpression", toJsonOrNull(_node.initializationExpression())),
 		std::make_pair("condition", toJsonOrNull(_node.condition())),
 		std::make_pair("loopExpression", toJsonOrNull(_node.loopExpression())),
 		std::make_pair("body", toJson(_node.body()))
-	});
+	};
+
+	if (_node.annotation().isSimpleCounterLoop.set())
+		attributes.emplace_back("isSimpleCounterLoop", *_node.annotation().isSimpleCounterLoop);
+
+	setJsonNode(_node, "ForStatement", std::move(attributes));
 	return false;
 }
 

libsolidity/codegen/ContractCompiler.cpp

@@ -1245,7 +1245,16 @@ bool ContractCompiler::visit(ForStatement const& _forStatement)
 
 	// for's loop expression if existing
 	if (_forStatement.loopExpression())
+	{
+		Arithmetic previousArithmetic = m_context.arithmetic();
+		if (
+			*_forStatement.annotation().isSimpleCounterLoop &&
+			m_optimiserSettings.simpleCounterForLoopUncheckedIncrement
+		)
+			m_context.setArithmetic(Arithmetic::Wrapping);
 		_forStatement.loopExpression()->accept(*this);
+		m_context.setArithmetic(previousArithmetic);
+	}
 
 	m_context.appendJumpTo(loopStart);
 

libsolidity/codegen/ir/IRGenerator.cpp

@@ -224,7 +224,7 @@ std::string IRGenerator::generate(
 
 std::string IRGenerator::generate(Block const& _block)
 {
-	IRGeneratorForStatements generator(m_context, m_utils);
+	IRGeneratorForStatements generator(m_context, m_utils, m_optimiserSettings);
 	generator.generate(_block);
 	return generator.code();
 }
@@ -450,7 +450,7 @@ std::string IRGenerator::generateModifier(
 			(!_modifierInvocation.arguments() || _modifierInvocation.arguments()->empty()),
 			""
 		);
-		IRGeneratorForStatements expressionEvaluator(m_context, m_utils);
+		IRGeneratorForStatements expressionEvaluator(m_context, m_utils, m_optimiserSettings);
 		if (_modifierInvocation.arguments())
 			for (size_t i = 0; i < _modifierInvocation.arguments()->size(); i++)
 			{
@@ -465,7 +465,7 @@ std::string IRGenerator::generateModifier(
 			}
 
 		t("evalArgs", expressionEvaluator.code());
-		IRGeneratorForStatements generator(m_context, m_utils, [&]() {
+		IRGeneratorForStatements generator(m_context, m_utils, m_optimiserSettings, [&]() {
 			std::string ret = joinHumanReadable(retParams);
 			return
 				(ret.empty() ? "" : ret + " := ") +
@@ -575,7 +575,7 @@ std::string IRGenerator::generateGetter(VariableDeclaration const& _varDecl)
 				dispenseLocationComment(m_context.mostDerivedContract())
 			)
 			("functionName", functionName)
-			("constantValueFunction", IRGeneratorForStatements(m_context, m_utils).constantValueFunction(_varDecl))
+			("constantValueFunction", IRGeneratorForStatements(m_context, m_utils, m_optimiserSettings).constantValueFunction(_varDecl))
 			("ret", suffixedVariableNameList("ret_", 0, _varDecl.type()->sizeOnStack()))
 			.render();
 		}
@@ -750,7 +750,7 @@ std::string IRGenerator::generateExternalFunction(ContractDefinition const& _con
 
 std::string IRGenerator::generateInitialAssignment(VariableDeclaration const& _varDecl)
 {
-	IRGeneratorForStatements generator(m_context, m_utils);
+	IRGeneratorForStatements generator(m_context, m_utils, m_optimiserSettings);
 	generator.initializeLocalVar(_varDecl);
 	return generator.code();
 }
@@ -799,7 +799,7 @@ std::pair<std::string, std::map<ContractDefinition const*, std::vector<std::stri
 						modifier->arguments()
 					).second, "");
 
-	IRGeneratorForStatements generator{m_context, m_utils};
+	IRGeneratorForStatements generator{m_context, m_utils, m_optimiserSettings};
 	for (auto&& [baseContract, arguments]: baseConstructorArguments)
 	{
 		solAssert(baseContract && arguments, "");
@@ -820,7 +820,7 @@ std::pair<std::string, std::map<ContractDefinition const*, std::vector<std::stri
 
 std::string IRGenerator::initStateVariables(ContractDefinition const& _contract)
 {
-	IRGeneratorForStatements generator{m_context, m_utils};
+	IRGeneratorForStatements generator{m_context, m_utils, m_optimiserSettings};
 	for (VariableDeclaration const* variable: _contract.stateVariables())
 		if (!variable->isConstant())
 			generator.initializeStateVar(*variable);

libsolidity/codegen/ir/IRGenerator.h

@@ -27,6 +27,7 @@
 #include <libsolidity/ast/CallGraph.h>
 #include <libsolidity/codegen/ir/IRGenerationContext.h>
 #include <libsolidity/codegen/YulUtilFunctions.h>
+#include <libsolidity/interface/OptimiserSettings.h>
 
 #include <liblangutil/CharStreamProvider.h>
 #include <liblangutil/EVMVersion.h>
@@ -51,7 +52,8 @@ class IRGenerator
 		RevertStrings _revertStrings,
 		std::map<std::string, unsigned> _sourceIndices,
 		langutil::DebugInfoSelection const& _debugInfoSelection,
-		langutil::CharStreamProvider const* _soliditySourceProvider
+		langutil::CharStreamProvider const* _soliditySourceProvider,
+		OptimiserSettings& _optimiserSettings
 	):
 		m_evmVersion(_evmVersion),
 		m_eofVersion(_eofVersion),
@@ -63,7 +65,8 @@ class IRGenerator
 			_debugInfoSelection,
 			_soliditySourceProvider
 		),
-		m_utils(_evmVersion, m_context.revertStrings(), m_context.functionCollector())
+		m_utils(_evmVersion, m_context.revertStrings(), m_context.functionCollector()),
+		m_optimiserSettings(_optimiserSettings)
 	{}
 
 	/// Generates and returns (unoptimized) IR code.
@@ -141,6 +144,7 @@ class IRGenerator
 
 	IRGenerationContext m_context;
 	YulUtilFunctions m_utils;
+	OptimiserSettings m_optimiserSettings;
 };
 
 }

libsolidity/codegen/ir/IRGeneratorForStatements.cpp

@@ -352,7 +352,7 @@ std::string IRGeneratorForStatements::constantValueFunction(VariableDeclaration
 			)");
 			templ("sourceLocationComment", dispenseLocationComment(_constant, m_context));
 			templ("functionName", functionName);
-			IRGeneratorForStatements generator(m_context, m_utils);
+			IRGeneratorForStatements generator(m_context, m_utils, m_optimiserSettings);
 			solAssert(_constant.value());
 			Type const& constantType = *_constant.type();
 			templ("value", generator.evaluateExpression(*_constant.value(), constantType).commaSeparatedList());
@@ -617,7 +617,9 @@ bool IRGeneratorForStatements::visit(ForStatement const& _forStatement)
 		_forStatement.body(),
 		_forStatement.condition(),
 		_forStatement.initializationExpression(),
-		_forStatement.loopExpression()
+		_forStatement.loopExpression(),
+		false, // _isDoWhile
+		*_forStatement.annotation().isSimpleCounterLoop
 	);
 
 	return false;
@@ -3192,7 +3194,8 @@ void IRGeneratorForStatements::generateLoop(
 	Expression const* _conditionExpression,
 	Statement const*  _initExpression,
 	ExpressionStatement const* _loopExpression,
-	bool _isDoWhile
+	bool _isDoWhile,
+	bool _isSimpleCounterLoop
 )
 {
 	std::string firstRun;
@@ -3209,7 +3212,13 @@ void IRGeneratorForStatements::generateLoop(
 		_initExpression->accept(*this);
 	appendCode() << "} 1 {\n";
 	if (_loopExpression)
+	{
+		Arithmetic previousArithmetic = m_context.arithmetic();
+		if (m_optimiserSettings.simpleCounterForLoopUncheckedIncrement && _isSimpleCounterLoop)
+			m_context.setArithmetic(Arithmetic::Wrapping);
 		_loopExpression->accept(*this);
+		m_context.setArithmetic(previousArithmetic);
+	}
 	appendCode() << "}\n";
 	appendCode() << "{\n";