Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add simplification rule for bitwise shifting #3580

Merged
merged 4 commits into from
Apr 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Features:
* 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 ``SHL`` and ``SHR`` only involving constants (Constantinople only).
* Optimizer: Remove useless ``SWAP1`` instruction preceding a commutative instruction (such as ``ADD``, ``MUL``, etc).
* Optimizer: Replace comparison operators (``LT``, ``GT``, etc) with opposites if preceded by ``SWAP1``, e.g. ``SWAP1 LT`` is replaced with ``GT``.
* Optimizer: Optimize across ``mload`` if ``msize()`` is not used.
Expand Down
10 changes: 10 additions & 0 deletions libevmasm/RuleList.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,16 @@ std::vector<SimplificationRule<Pattern>> simplificationRuleList(
u256 mask = (u256(1) << testBit) - 1;
return u256(boost::multiprecision::bit_test(B.d(), testBit) ? B.d() | ~mask : B.d() & mask);
}, false},
{{Instruction::SHL, {A, B}}, [=]{
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chriseth do we want to introduce evmVersion in the rule list or is it safe to have this merged (after some cleanup/squashing)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be safe. Let's add a comment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about the order of the operands?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(is that possible?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is blocked by the same problem as in #3797: need to build a constantinople supporting version of eth and run endtoend tests with that target.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about the order of the operands?

The top of the stack is the number of bits to shift he value with.

if (A.d() > 255)
return u256(0);
return u256(bigint(B.d()) << unsigned(A.d()));
}, false},
{{Instruction::SHR, {A, B}}, [=]{
if (A.d() > 255)
return u256(0);
return B.d() >> unsigned(A.d());
}, false},

// invariants involving known constants
{{Instruction::ADD, {X, 0}}, [=]{ return X; }, false},
Expand Down
9 changes: 8 additions & 1 deletion scripts/tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,18 @@ then
progress=""
fi

EVM_VERSIONS="homestead byzantium"

if [ "$CIRCLECI" ] || [ -z "$CI" ]
then
EVM_VERSIONS+=" constantinople"
fi

# And then run the Solidity unit-tests in the matrix combination of optimizer / no optimizer
# and homestead / byzantium VM, # pointing to that IPC endpoint.
for optimize in "" "--optimize"
do
for vm in homestead byzantium
for vm in $EVM_VERSIONS
do
echo "--> Running tests using "$optimize" --evm-version "$vm"..."
log=""
Expand Down
2 changes: 2 additions & 0 deletions test/libsolidity/InlineAssembly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,8 @@ BOOST_AUTO_TEST_CASE(shift)

BOOST_AUTO_TEST_CASE(shift_constantinople_warning)
{
if (dev::test::Options::get().evmVersion().hasBitwiseShifting())
return;
CHECK_PARSE_WARNING("{ pop(shl(10, 32)) }", Warning, "The \"shl\" instruction is only available for Constantinople-compatible VMs.");
CHECK_PARSE_WARNING("{ pop(shr(10, 32)) }", Warning, "The \"shr\" instruction is only available for Constantinople-compatible VMs.");
CHECK_PARSE_WARNING("{ pop(sar(10, 32)) }", Warning, "The \"sar\" instruction is only available for Constantinople-compatible VMs.");
Expand Down
129 changes: 129 additions & 0 deletions test/libsolidity/SolidityEndToEndTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11141,6 +11141,135 @@ BOOST_AUTO_TEST_CASE(swap_peephole_optimisation)
BOOST_CHECK(callContractFunction("div(uint256,uint256)", u256(0), u256(1)) == encodeArgs(u256(0)));
}

BOOST_AUTO_TEST_CASE(bitwise_shifting_constantinople)
{
if (!dev::test::Options::get().evmVersion().hasBitwiseShifting())
return;
char const* sourceCode = R"(
contract C {
function shl(uint a, uint b) returns (uint c) {
assembly {
a
b
shl
=: c
}
}
function shr(uint a, uint b) returns (uint c) {
assembly {
a
b
shr
=: c
}
}
function sar(uint a, uint b) returns (uint c) {
assembly {
a
b
sar
=: c
}
}
}
)";
compileAndRun(sourceCode);
BOOST_CHECK(callContractFunction("shl(uint256,uint256)", u256(1), u256(2)) == encodeArgs(u256(4)));
BOOST_CHECK(callContractFunction("shl(uint256,uint256)", u256("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), u256(1)) == encodeArgs(u256("0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe")));
BOOST_CHECK(callContractFunction("shl(uint256,uint256)", u256("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), u256(256)) == encodeArgs(u256(0)));
BOOST_CHECK(callContractFunction("shr(uint256,uint256)", u256(3), u256(1)) == encodeArgs(u256(1)));
BOOST_CHECK(callContractFunction("shr(uint256,uint256)", u256("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), u256(1)) == encodeArgs(u256("0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")));
BOOST_CHECK(callContractFunction("shr(uint256,uint256)", u256("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), u256(255)) == encodeArgs(u256(1)));
BOOST_CHECK(callContractFunction("shr(uint256,uint256)", u256("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), u256(256)) == encodeArgs(u256(0)));
BOOST_CHECK(callContractFunction("sar(uint256,uint256)", u256(3), u256(1)) == encodeArgs(u256(1)));
BOOST_CHECK(callContractFunction("sar(uint256,uint256)", u256("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), u256(1)) == encodeArgs(u256("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")));
BOOST_CHECK(callContractFunction("sar(uint256,uint256)", u256("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), u256(255)) == encodeArgs(u256("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")));
BOOST_CHECK(callContractFunction("sar(uint256,uint256)", u256("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), u256(256)) == encodeArgs(u256("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")));
}

BOOST_AUTO_TEST_CASE(bitwise_shifting_constants_constantinople)
{
if (!dev::test::Options::get().evmVersion().hasBitwiseShifting())
return;
char const* sourceCode = R"(
contract C {
function shl_1() returns (bool) {
uint c;
assembly {
1
2
shl
=: c
}
assert(c == 4);
return true;
}
function shl_2() returns (bool) {
uint c;
assembly {
0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
1
shl
=: c
}
assert(c == 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe);
return true;
}
function shl_3() returns (bool) {
uint c;
assembly {
0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
256
shl
=: c
}
assert(c == 0);
return true;
}
function shr_1() returns (bool) {
uint c;
assembly {
3
1
shr
=: c
}
assert(c == 1);
return true;
}
function shr_2() returns (bool) {
uint c;
assembly {
0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
1
shr
=: c
}
assert(c == 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff);
return true;
}
function shr_3() returns (bool) {
uint c;
assembly {
0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
256
shr
=: c
}
assert(c == 0);
return true;
}
}
)";
compileAndRun(sourceCode);
BOOST_CHECK(callContractFunction("shl_1()") == encodeArgs(u256(1)));
BOOST_CHECK(callContractFunction("shl_2()") == encodeArgs(u256(1)));
BOOST_CHECK(callContractFunction("shl_3()") == encodeArgs(u256(1)));
BOOST_CHECK(callContractFunction("shr_1()") == encodeArgs(u256(1)));
BOOST_CHECK(callContractFunction("shr_2()") == encodeArgs(u256(1)));
BOOST_CHECK(callContractFunction("shr_3()") == encodeArgs(u256(1)));
}

BOOST_AUTO_TEST_SUITE_END()

}
Expand Down