Merge pull request #3693 from ethereum/optimizeMLOAD

Optimize across MLOAD if MSIZE is not used.
This commit is contained in:
Alex Beregszaszi 2018-04-03 15:58:11 +01:00 committed by GitHub
commit 0edce4b570
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 148 additions and 42 deletions

View File

@ -1,9 +1,11 @@
### 0.4.22 (unreleased)
Features:
* Code Generator: Initialize arrays without using ``msize()``.
* Commandline interface: Error when missing or inaccessible file detected. Suppress it with the ``--ignore-missing`` flag.
* General: Support accessing dynamic return data in post-byzantium EVMs.
* Interfaces: Allow overriding external functions in interfaces with public in an implementing contract.
* Optimizer: Optimize across ``mload`` if ``msize()`` is not used.
* Syntax Checker: Issue warning for empty structs (or error as experimental 0.5.0 feature).
Bugfixes:

View File

@ -438,13 +438,15 @@ map<u256, u256> Assembly::optimiseInternal(
// function types that can be stored in storage.
AssemblyItems optimisedItems;
bool usesMSize = (find(m_items.begin(), m_items.end(), AssemblyItem(Instruction::MSIZE)) != m_items.end());
auto iter = m_items.begin();
while (iter != m_items.end())
{
KnownState emptyState;
CommonSubexpressionEliminator eliminator(emptyState);
auto orig = iter;
iter = eliminator.feedItems(iter, m_items.end());
iter = eliminator.feedItems(iter, m_items.end(), usesMSize);
bool shouldReplace = false;
AssemblyItems optimisedChunk;
try

View File

@ -65,8 +65,9 @@ public:
/// Feeds AssemblyItems into the eliminator and @returns the iterator pointing at the first
/// item that must be fed into a new instance of the eliminator.
/// @param _msizeImportant if false, do not consider modification of MSIZE a side-effect
template <class _AssemblyItemIterator>
_AssemblyItemIterator feedItems(_AssemblyItemIterator _iterator, _AssemblyItemIterator _end);
_AssemblyItemIterator feedItems(_AssemblyItemIterator _iterator, _AssemblyItemIterator _end, bool _msizeImportant);
/// @returns the resulting items after optimization.
AssemblyItems getOptimizedItems();
@ -168,11 +169,12 @@ private:
template <class _AssemblyItemIterator>
_AssemblyItemIterator CommonSubexpressionEliminator::feedItems(
_AssemblyItemIterator _iterator,
_AssemblyItemIterator _end
_AssemblyItemIterator _end,
bool _msizeImportant
)
{
assertThrow(!m_breakingItem, OptimizerException, "Invalid use of CommonSubexpressionEliminator.");
for (; _iterator != _end && !SemanticInformation::breaksCSEAnalysisBlock(*_iterator); ++_iterator)
for (; _iterator != _end && !SemanticInformation::breaksCSEAnalysisBlock(*_iterator, _msizeImportant); ++_iterator)
feedItem(*_iterator);
if (_iterator != _end)
m_breakingItem = &(*_iterator++);

View File

@ -199,7 +199,7 @@ static const std::map<Instruction, InstructionInfo> c_instructionInfo =
{ Instruction::ADDMOD, { "ADDMOD", 0, 3, 1, false, Tier::Mid } },
{ Instruction::MULMOD, { "MULMOD", 0, 3, 1, false, Tier::Mid } },
{ Instruction::SIGNEXTEND, { "SIGNEXTEND", 0, 2, 1, false, Tier::Low } },
{ Instruction::KECCAK256, { "KECCAK256", 0, 2, 1, false, Tier::Special } },
{ Instruction::KECCAK256, { "KECCAK256", 0, 2, 1, true, Tier::Special } },
{ Instruction::ADDRESS, { "ADDRESS", 0, 0, 1, false, Tier::Base } },
{ Instruction::BALANCE, { "BALANCE", 0, 1, 1, false, Tier::Balance } },
{ Instruction::ORIGIN, { "ORIGIN", 0, 0, 1, false, Tier::Base } },

View File

@ -28,7 +28,7 @@ using namespace std;
using namespace dev;
using namespace dev::eth;
bool SemanticInformation::breaksCSEAnalysisBlock(AssemblyItem const& _item)
bool SemanticInformation::breaksCSEAnalysisBlock(AssemblyItem const& _item, bool _msizeImportant)
{
switch (_item.type())
{
@ -59,6 +59,11 @@ bool SemanticInformation::breaksCSEAnalysisBlock(AssemblyItem const& _item)
return false;
if (_item.instruction() == Instruction::MSTORE)
return false;
if (!_msizeImportant && (
_item.instruction() == Instruction::MLOAD ||
_item.instruction() == Instruction::KECCAK256
))
return false;
//@todo: We do not handle the following memory instructions for now:
// calldatacopy, codecopy, extcodecopy, mstore8,
// msize (note that msize also depends on memory read access)

View File

@ -38,7 +38,8 @@ class AssemblyItem;
struct SemanticInformation
{
/// @returns true if the given items starts a new block for common subexpression analysis.
static bool breaksCSEAnalysisBlock(AssemblyItem const& _item);
/// @param _msizeImportant if false, consider an operation non-breaking if its only side-effect is that it modifies msize.
static bool breaksCSEAnalysisBlock(AssemblyItem const& _item, bool _msizeImportant);
/// @returns true if the item is a two-argument operation whose value does not depend on the
/// order of its arguments.
static bool isCommutativeOperation(AssemblyItem const& _item);

View File

@ -229,6 +229,9 @@ public:
/// i.e. it behaves differently in lvalue context and in value context.
virtual bool isValueType() const { return false; }
virtual unsigned sizeOnStack() const { return 1; }
/// If it is possible to initialize such a value in memory by just writing zeros
/// of the size memoryHeadSize().
virtual bool hasSimpleZeroValueInMemory() const { return true; }
/// @returns the mobile (in contrast to static) type corresponding to the given type.
/// This returns the corresponding IntegerType or FixedPointType for RationalNumberType
/// and the pointer type for storage reference types.
@ -568,6 +571,7 @@ public:
virtual TypePointer mobileType() const override { return copyForLocation(m_location, true); }
virtual bool dataStoredIn(DataLocation _location) const override { return m_location == _location; }
virtual bool hasSimpleZeroValueInMemory() const override { return false; }
/// Storage references can be pointers or bound references. In general, local variables are of
/// pointer type, state variables are bound references. Assignments to pointers or deleting
@ -855,6 +859,7 @@ public:
virtual u256 storageSize() const override;
virtual bool canLiveOutsideStorage() const override { return false; }
virtual unsigned sizeOnStack() const override;
virtual bool hasSimpleZeroValueInMemory() const override { return false; }
virtual TypePointer mobileType() const override;
/// Converts components to their temporary types and performs some wildcard matching.
virtual TypePointer closestTemporaryType(TypePointer const& _targetType) const override;
@ -999,6 +1004,7 @@ public:
virtual bool isValueType() const override { return true; }
virtual bool canLiveOutsideStorage() const override { return m_kind == Kind::Internal || m_kind == Kind::External; }
virtual unsigned sizeOnStack() const override;
virtual bool hasSimpleZeroValueInMemory() const override { return false; }
virtual MemberList::MemberMap nativeMembers(ContractDefinition const* _currentScope) const override;
virtual TypePointer encodingType() const override;
virtual TypePointer interfaceType(bool _inLibrary) const override;
@ -1104,6 +1110,8 @@ public:
return _inLibrary ? shared_from_this() : TypePointer();
}
virtual bool dataStoredIn(DataLocation _location) const override { return _location == DataLocation::Storage; }
/// Cannot be stored in memory, but just in case.
virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); }
TypePointer const& keyType() const { return m_keyType; }
TypePointer const& valueType() const { return m_valueType; }
@ -1132,6 +1140,7 @@ public:
virtual u256 storageSize() const override;
virtual bool canLiveOutsideStorage() const override { return false; }
virtual unsigned sizeOnStack() const override;
virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); }
virtual std::string toString(bool _short) const override { return "type(" + m_actualType->toString(_short) + ")"; }
virtual MemberList::MemberMap nativeMembers(ContractDefinition const* _currentScope) const override;
@ -1154,6 +1163,7 @@ public:
virtual u256 storageSize() const override;
virtual bool canLiveOutsideStorage() const override { return false; }
virtual unsigned sizeOnStack() const override { return 0; }
virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); }
virtual std::string richIdentifier() const override;
virtual bool operator==(Type const& _other) const override;
virtual std::string toString(bool _short) const override;
@ -1179,6 +1189,7 @@ public:
virtual bool operator==(Type const& _other) const override;
virtual bool canBeStored() const override { return false; }
virtual bool canLiveOutsideStorage() const override { return true; }
virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); }
virtual unsigned sizeOnStack() const override { return 0; }
virtual MemberList::MemberMap nativeMembers(ContractDefinition const*) const override;
@ -1209,6 +1220,7 @@ public:
virtual bool operator==(Type const& _other) const override;
virtual bool canBeStored() const override { return false; }
virtual bool canLiveOutsideStorage() const override { return true; }
virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); }
virtual unsigned sizeOnStack() const override { return 0; }
virtual MemberList::MemberMap nativeMembers(ContractDefinition const*) const override;
@ -1238,6 +1250,7 @@ public:
virtual bool canLiveOutsideStorage() const override { return false; }
virtual bool isValueType() const override { return true; }
virtual unsigned sizeOnStack() const override { return 1; }
virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); }
virtual std::string toString(bool) const override { return "inaccessible dynamic type"; }
virtual TypePointer decodingType() const override { return std::make_shared<IntegerType>(256); }
};

View File

@ -495,6 +495,25 @@ void CompilerUtils::abiDecodeV2(TypePointers const& _parameterTypes, bool _fromM
void CompilerUtils::zeroInitialiseMemoryArray(ArrayType const& _type)
{
if (_type.baseType()->hasSimpleZeroValueInMemory())
{
solAssert(_type.baseType()->isValueType(), "");
Whiskers templ(R"({
let size := mul(length, <element_size>)
// cheap way of zero-initializing a memory range
codecopy(memptr, codesize(), size)
memptr := add(memptr, size)
})");
templ("element_size", to_string(_type.baseType()->memoryHeadSize()));
m_context.appendInlineAssembly(templ.render(), {"length", "memptr"});
}
else
{
// TODO: Potential optimization:
// When we create a new multi-dimensional dynamic array, each element
// is initialized to an empty array. It actually does not hurt
// to re-use exactly the same empty array for all elements. Currently,
// a new one is created each time.
auto repeat = m_context.newTag();
m_context << repeat;
pushZeroValue(*_type.baseType());
@ -503,6 +522,7 @@ void CompilerUtils::zeroInitialiseMemoryArray(ArrayType const& _type)
m_context << Instruction::SUB << Instruction::SWAP1;
m_context << Instruction::DUP2;
m_context.appendConditionalJumpTo(repeat);
}
m_context << Instruction::SWAP1 << Instruction::POP;
}

View File

@ -850,8 +850,6 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
}
case FunctionType::Kind::ObjectCreation:
{
// Will allocate at the end of memory (MSIZE) and not write at all unless the base
// type is dynamically sized.
ArrayType const& arrayType = dynamic_cast<ArrayType const&>(*_functionCall.annotation().type);
_functionCall.expression().accept(*this);
solAssert(arguments.size() == 1, "");
@ -861,15 +859,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
utils().convertType(*arguments[0]->annotation().type, IntegerType(256));
// Stack: requested_length
// Allocate at max(MSIZE, freeMemoryPointer)
utils().fetchFreeMemoryPointer();
m_context << Instruction::DUP1 << Instruction::MSIZE;
m_context << Instruction::LT;
auto initialise = m_context.appendConditionalJump();
// Free memory pointer does not point to empty memory, use MSIZE.
m_context << Instruction::POP;
m_context << Instruction::MSIZE;
m_context << initialise;
// Stack: requested_length memptr
m_context << Instruction::SWAP1;
@ -894,13 +884,10 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
// Check if length is zero
m_context << Instruction::DUP1 << Instruction::ISZERO;
auto skipInit = m_context.appendConditionalJump();
// We only have to initialise if the base type is a not a value type.
if (dynamic_cast<ReferenceType const*>(arrayType.baseType().get()))
{
// Always initialize because the free memory pointer might point at
// a dirty memory area.
m_context << Instruction::DUP2 << u256(32) << Instruction::ADD;
utils().zeroInitialiseMemoryArray(arrayType);
}
m_context << skipInit;
m_context << Instruction::POP;
break;

View File

@ -69,8 +69,9 @@ namespace
{
AssemblyItems input = addDummyLocations(_input);
bool usesMsize = (find(_input.begin(), _input.end(), AssemblyItem{Instruction::MSIZE}) != _input.end());
eth::CommonSubexpressionEliminator cse(_state);
BOOST_REQUIRE(cse.feedItems(input.begin(), input.end()) == input.end());
BOOST_REQUIRE(cse.feedItems(input.begin(), input.end(), usesMsize) == input.end());
AssemblyItems output = cse.getOptimizedItems();
for (AssemblyItem const& item: output)
@ -124,7 +125,7 @@ BOOST_AUTO_TEST_CASE(cse_intermediate_swap)
Instruction::SLOAD, Instruction::SWAP1, u256(100), Instruction::EXP, Instruction::SWAP1,
Instruction::DIV, u256(0xff), Instruction::AND
};
BOOST_REQUIRE(cse.feedItems(input.begin(), input.end()) == input.end());
BOOST_REQUIRE(cse.feedItems(input.begin(), input.end(), false) == input.end());
AssemblyItems output = cse.getOptimizedItems();
BOOST_CHECK(!output.empty());
}

View File

@ -8756,6 +8756,32 @@ BOOST_AUTO_TEST_CASE(create_dynamic_array_with_zero_length)
ABI_CHECK(callContractFunction("f()"), encodeArgs(u256(7)));
}
BOOST_AUTO_TEST_CASE(correctly_initialize_memory_array_in_constructor)
{
// Memory arrays are initialized using codecopy past the size of the code.
// This test checks that it also works in the constructor context.
char const* sourceCode = R"(
contract C {
bool public success;
function C() public {
// Make memory dirty.
assembly {
for { let i := 0 } lt(i, 64) { i := add(i, 1) } {
mstore(msize, not(0))
}
}
uint16[3] memory c;
require(c[0] == 0 && c[1] == 0 && c[2] == 0);
uint16[] memory x = new uint16[](3);
require(x[0] == 0 && x[1] == 0 && x[2] == 0);
success = true;
}
}
)";
compileAndRun(sourceCode, 0, "C");
ABI_CHECK(callContractFunction("success()"), encodeArgs(u256(1)));
}
BOOST_AUTO_TEST_CASE(return_does_not_skip_modifier)
{
char const* sourceCode = R"(
@ -9119,7 +9145,7 @@ BOOST_AUTO_TEST_CASE(calling_uninitialized_function_in_detail)
int mutex;
function t() returns (uint) {
if (mutex > 0)
return 7;
{ assembly { mstore(0, 7) return(0, 0x20) } }
mutex = 1;
// Avoid re-executing this function if we jump somewhere.
x();
@ -9132,6 +9158,27 @@ BOOST_AUTO_TEST_CASE(calling_uninitialized_function_in_detail)
ABI_CHECK(callContractFunction("t()"), encodeArgs());
}
BOOST_AUTO_TEST_CASE(calling_uninitialized_function_through_array)
{
char const* sourceCode = R"(
contract C {
int mutex;
function t() returns (uint) {
if (mutex > 0)
{ assembly { mstore(0, 7) return(0, 0x20) } }
mutex = 1;
// Avoid re-executing this function if we jump somewhere.
function() internal returns (uint)[200] x;
x[0]();
return 2;
}
}
)";
compileAndRun(sourceCode, 0, "C");
ABI_CHECK(callContractFunction("t()"), encodeArgs());
}
BOOST_AUTO_TEST_CASE(pass_function_types_internally)
{
char const* sourceCode = R"(

View File

@ -74,11 +74,11 @@ public:
unsigned const _optimizeRuns = 200
)
{
bytes nonOptimizedBytecode = compileAndRunWithOptimizer(_sourceCode, _value, _contractName, false, _optimizeRuns);
m_nonOptimizedBytecode = compileAndRunWithOptimizer(_sourceCode, _value, _contractName, false, _optimizeRuns);
m_nonOptimizedContract = m_contractAddress;
bytes optimizedBytecode = compileAndRunWithOptimizer(_sourceCode, _value, _contractName, true, _optimizeRuns);
size_t nonOptimizedSize = numInstructions(nonOptimizedBytecode);
size_t optimizedSize = numInstructions(optimizedBytecode);
m_optimizedBytecode = compileAndRunWithOptimizer(_sourceCode, _value, _contractName, true, _optimizeRuns);
size_t nonOptimizedSize = numInstructions(m_nonOptimizedBytecode);
size_t optimizedSize = numInstructions(m_optimizedBytecode);
BOOST_CHECK_MESSAGE(
_optimizeRuns < 50 || optimizedSize < nonOptimizedSize,
string("Optimizer did not reduce bytecode size. Non-optimized size: ") +
@ -104,7 +104,7 @@ public:
/// @returns the number of intructions in the given bytecode, not taking the metadata hash
/// into account.
size_t numInstructions(bytes const& _bytecode)
size_t numInstructions(bytes const& _bytecode, boost::optional<Instruction> _which = boost::optional<Instruction>{})
{
BOOST_REQUIRE(_bytecode.size() > 5);
size_t metadataSize = (_bytecode[_bytecode.size() - 2] << 8) + _bytecode[_bytecode.size() - 1];
@ -112,13 +112,16 @@ public:
BOOST_REQUIRE(_bytecode.size() >= metadataSize + 2);
bytes realCode = bytes(_bytecode.begin(), _bytecode.end() - metadataSize - 2);
size_t instructions = 0;
solidity::eachInstruction(realCode, [&](Instruction, u256 const&) {
solidity::eachInstruction(realCode, [&](Instruction _instr, u256 const&) {
if (!_which || *_which == _instr)
instructions++;
});
return instructions;
}
protected:
bytes m_nonOptimizedBytecode;
bytes m_optimizedBytecode;
Address m_optimizedContract;
Address m_nonOptimizedContract;
};
@ -581,6 +584,29 @@ BOOST_AUTO_TEST_CASE(invalid_state_at_control_flow_join)
compareVersions("test()");
}
BOOST_AUTO_TEST_CASE(optimise_multi_stores)
{
char const* sourceCode = R"(
contract Test {
struct S { uint16 a; uint16 b; uint16[3] c; uint[] dyn; }
uint padding;
S[] s;
function f() public returns (uint16, uint16, uint16[3], uint) {
uint16[3] memory c;
c[0] = 7;
c[1] = 8;
c[2] = 9;
s.push(S(1, 2, c, new uint[](4)));
return (s[0].a, s[0].b, s[0].c, s[0].dyn[2]);
}
}
)";
compileBothVersions(sourceCode);
compareVersions("f()");
BOOST_CHECK_EQUAL(numInstructions(m_nonOptimizedBytecode, Instruction::SSTORE), 13);
BOOST_CHECK_EQUAL(numInstructions(m_optimizedBytecode, Instruction::SSTORE), 11);
}
BOOST_AUTO_TEST_SUITE_END()
}