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

Expression Inliner #3236

Merged
merged 7 commits into from Feb 7, 2018

Conversation

Projects
None yet
2 participants
@chriseth
Contributor

chriseth commented Nov 22, 2017

Depends on #3219, #3279 and #3352.

@chriseth

This comment has been minimized.

Show comment
Hide comment
@chriseth

chriseth Nov 24, 2017

Contributor

This is now able to turn

{
    function abi_encode_t_contract$_Fund_$93_to_t_address(value, pos)
    {
        mstore(pos, convert_t_contract$_Fund_$93_to_t_address(value))
    }
    function abi_encode_t_rational_10_by_1_to_t_uint256(value, pos)
    {
        mstore(pos, convert_t_rational_10_by_1_to_t_uint256(value))
    }
    function abi_encode_t_uint256_to_t_uint256(value, pos)
    {
        mstore(pos, cleanup_assert_t_uint256(value))
    }
    function abi_encode_tuple_t_contract$_Fund_$93__to_t_address_(headStart, value0) -> tail
    {
        tail := add(headStart, 32)
        abi_encode_t_contract$_Fund_$93_to_t_address(value0, add(headStart, 0))
    }
    function abi_encode_tuple_t_rational_10_by_1__to_t_uint256_(headStart, value0) -> tail
    {
        tail := add(headStart, 32)
        abi_encode_t_rational_10_by_1_to_t_uint256(value0, add(headStart, 0))
    }
    function abi_encode_tuple_t_uint256__to_t_uint256_(headStart, value0) -> tail
    {
        tail := add(headStart, 32)
        abi_encode_t_uint256_to_t_uint256(value0, add(headStart, 0))
    }
    function cleanup_assert_t_address(value) -> cleaned
    {
        cleaned := and(value, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
    }
    function cleanup_assert_t_uint256(value) -> cleaned
    {
        cleaned := value
    }
    function convert_t_contract$_Fund_$93_to_t_address(value) -> converted
    {
        converted := cleanup_assert_t_address(value)
    }
    function convert_t_rational_10_by_1_to_t_uint256(value) -> converted
    {
        converted := cleanup_assert_t_uint256(value)
    }
}

into

{
    function abi_encode_t_contract$_Fund_$93_to_t_address(value, pos)
    {
        mstore(pos, and(value, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF))
    }
    function abi_encode_t_rational_10_by_1_to_t_uint256(value_1, pos_1)
    {
        mstore(pos_1, value_1)
    }
    function abi_encode_t_uint256_to_t_uint256(value_2, pos_2)
    {
        mstore(pos_2, value_2)
    }
    function abi_encode_tuple_t_contract$_Fund_$93__to_t_address_(headStart, value0) -> tail
    {
        tail := add(headStart, 32)
        abi_encode_t_contract$_Fund_$93_to_t_address(value0, add(headStart, 0))
    }
    function abi_encode_tuple_t_rational_10_by_1__to_t_uint256_(headStart_1, value0_1) -> tail_1
    {
        tail_1 := add(headStart_1, 32)
        abi_encode_t_rational_10_by_1_to_t_uint256(value0_1, add(headStart_1, 0))
    }
    function abi_encode_tuple_t_uint256__to_t_uint256_(headStart_2, value0_2) -> tail_2
    {
        tail_2 := add(headStart_2, 32)
        abi_encode_t_uint256_to_t_uint256(value0_2, add(headStart_2, 0))
    }
    function cleanup_assert_t_address(value_3) -> cleaned
    {
        cleaned := and(value_3, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
    }
    function cleanup_assert_t_uint256(value_4) -> cleaned_1
    {
        cleaned_1 := value_4
    }
    function convert_t_contract$_Fund_$93_to_t_address(value_5) -> converted
    {
        converted := and(value_5, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
    }
    function convert_t_rational_10_by_1_to_t_uint256(value_6) -> converted_1
    {
        converted_1 := value_6
    }
}
Contributor

chriseth commented Nov 24, 2017

This is now able to turn

{
    function abi_encode_t_contract$_Fund_$93_to_t_address(value, pos)
    {
        mstore(pos, convert_t_contract$_Fund_$93_to_t_address(value))
    }
    function abi_encode_t_rational_10_by_1_to_t_uint256(value, pos)
    {
        mstore(pos, convert_t_rational_10_by_1_to_t_uint256(value))
    }
    function abi_encode_t_uint256_to_t_uint256(value, pos)
    {
        mstore(pos, cleanup_assert_t_uint256(value))
    }
    function abi_encode_tuple_t_contract$_Fund_$93__to_t_address_(headStart, value0) -> tail
    {
        tail := add(headStart, 32)
        abi_encode_t_contract$_Fund_$93_to_t_address(value0, add(headStart, 0))
    }
    function abi_encode_tuple_t_rational_10_by_1__to_t_uint256_(headStart, value0) -> tail
    {
        tail := add(headStart, 32)
        abi_encode_t_rational_10_by_1_to_t_uint256(value0, add(headStart, 0))
    }
    function abi_encode_tuple_t_uint256__to_t_uint256_(headStart, value0) -> tail
    {
        tail := add(headStart, 32)
        abi_encode_t_uint256_to_t_uint256(value0, add(headStart, 0))
    }
    function cleanup_assert_t_address(value) -> cleaned
    {
        cleaned := and(value, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
    }
    function cleanup_assert_t_uint256(value) -> cleaned
    {
        cleaned := value
    }
    function convert_t_contract$_Fund_$93_to_t_address(value) -> converted
    {
        converted := cleanup_assert_t_address(value)
    }
    function convert_t_rational_10_by_1_to_t_uint256(value) -> converted
    {
        converted := cleanup_assert_t_uint256(value)
    }
}

into

{
    function abi_encode_t_contract$_Fund_$93_to_t_address(value, pos)
    {
        mstore(pos, and(value, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF))
    }
    function abi_encode_t_rational_10_by_1_to_t_uint256(value_1, pos_1)
    {
        mstore(pos_1, value_1)
    }
    function abi_encode_t_uint256_to_t_uint256(value_2, pos_2)
    {
        mstore(pos_2, value_2)
    }
    function abi_encode_tuple_t_contract$_Fund_$93__to_t_address_(headStart, value0) -> tail
    {
        tail := add(headStart, 32)
        abi_encode_t_contract$_Fund_$93_to_t_address(value0, add(headStart, 0))
    }
    function abi_encode_tuple_t_rational_10_by_1__to_t_uint256_(headStart_1, value0_1) -> tail_1
    {
        tail_1 := add(headStart_1, 32)
        abi_encode_t_rational_10_by_1_to_t_uint256(value0_1, add(headStart_1, 0))
    }
    function abi_encode_tuple_t_uint256__to_t_uint256_(headStart_2, value0_2) -> tail_2
    {
        tail_2 := add(headStart_2, 32)
        abi_encode_t_uint256_to_t_uint256(value0_2, add(headStart_2, 0))
    }
    function cleanup_assert_t_address(value_3) -> cleaned
    {
        cleaned := and(value_3, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
    }
    function cleanup_assert_t_uint256(value_4) -> cleaned_1
    {
        cleaned_1 := value_4
    }
    function convert_t_contract$_Fund_$93_to_t_address(value_5) -> converted
    {
        converted := and(value_5, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
    }
    function convert_t_rational_10_by_1_to_t_uint256(value_6) -> converted_1
    {
        converted_1 := value_6
    }
}
@chriseth

This comment has been minimized.

Show comment
Hide comment
@chriseth

chriseth Nov 24, 2017

Contributor

Unused functions are not yet removed.

Contributor

chriseth commented Nov 24, 2017

Unused functions are not yet removed.

@chriseth chriseth referenced this pull request Nov 29, 2017

Merged

Full inliner #3256

@chriseth

This comment has been minimized.

Show comment
Hide comment
@chriseth

chriseth Dec 5, 2017

Contributor

I think I will re-implement part of this using the new tools, but please feel free to review the general idea.

Contributor

chriseth commented Dec 5, 2017

I think I will re-implement part of this using the new tools, but please feel free to review the general idea.

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Dec 8, 2017

Member

Shouldn't the FunctionalInstruction movable check be moved into the filter instead? It makes more sense to make the decision of inlineable function at once.

Member

axic commented Dec 8, 2017

Shouldn't the FunctionalInstruction movable check be moved into the filter instead? It makes more sense to make the decision of inlineable function at once.

@chriseth

This comment has been minimized.

Show comment
Hide comment
@chriseth

chriseth Dec 15, 2017

Contributor

The filter is about finding functions that can be inlined because of their structure. The movable check applies to the function call (more specifically, the arguments passed to the function), though, and not the function itself.

Contributor

chriseth commented Dec 15, 2017

The filter is about finding functions that can be inlined because of their structure. The movable check applies to the function call (more specifically, the arguments passed to the function), though, and not the function itself.

@chriseth chriseth changed the title from [WIP] Function inliner to Expression inliner Feb 6, 2018

@chriseth chriseth changed the title from Expression inliner to Expression Inliner Feb 6, 2018

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Feb 6, 2018

Member

Can you also rebase since the rematerialiser was merged?

Member

axic commented Feb 6, 2018

Can you also rebase since the rematerialiser was merged?

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Feb 6, 2018

Member

Rebased.

Member

axic commented Feb 6, 2018

Rebased.

@axic

axic approved these changes Feb 6, 2018

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Feb 6, 2018

Member

This is weird, compiles properly locally, but fails on travis:

gcc:

/home/travis/build/ethereum/solidity/test/libjulia/Inliner.cpp:73:39: error: unterminated raw string
  BOOST_CHECK_EQUAL(inlinableFunctions(R"({
                                       ^

clang:

/home/travis/build/ethereum/solidity/test/libjulia/Inliner.cpp:76:8: error: missing terminating '"' character [-Werror,-Winvalid-pp-token]
        })"), "inlinableFunctions(R\"({
              ^
Member

axic commented Feb 6, 2018

This is weird, compiles properly locally, but fails on travis:

gcc:

/home/travis/build/ethereum/solidity/test/libjulia/Inliner.cpp:73:39: error: unterminated raw string
  BOOST_CHECK_EQUAL(inlinableFunctions(R"({
                                       ^

clang:

/home/travis/build/ethereum/solidity/test/libjulia/Inliner.cpp:76:8: error: missing terminating '"' character [-Werror,-Winvalid-pp-token]
        })"), "inlinableFunctions(R\"({
              ^
@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Feb 6, 2018

Member

Still fails, there are still locations in the file failing.

Member

axic commented Feb 6, 2018

Still fails, there are still locations in the file failing.

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Feb 7, 2018

Member

Only has the "usual Zeppelin failures" now.

Member

axic commented Feb 7, 2018

Only has the "usual Zeppelin failures" now.

@axic axic merged commit 63fb319 into develop Feb 7, 2018

1 of 3 checks passed

ci/circleci Your tests failed on CircleCI
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@axic axic deleted the inliner branch Feb 7, 2018

@axic axic removed the in progress label Feb 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment