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

[YulOpti] Evaluate keccak256 whenever possible #11018

Closed
hrkrshnn opened this issue Feb 25, 2021 · 12 comments
Closed

[YulOpti] Evaluate keccak256 whenever possible #11018

hrkrshnn opened this issue Feb 25, 2021 · 12 comments
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. optimizer stale The issue/PR was marked as stale because it has been open for too long.
Projects

Comments

@hrkrshnn
Copy link
Member

For example, if we have a storage variable uint[] a, the following is the optimized IR code for computing arr[10] = 5:

Here value of _1 is 0.

mstore(_1, 0x01)
sstore(add(keccak256(_1, 0x20), 0x0a), 0x05)

It would be nice if we could compute this hash, and also avoid the mstore.

Was bought by in the forum by @frangio https://forum.soliditylang.org/t/enable-disable-language-features-to-save-gas/84/20

@hrkrshnn hrkrshnn added this to Inbox in Yul via automation Feb 25, 2021
@chriseth
Copy link
Contributor

The libevmasm optimizer can do it. I think it should actually compute it in this case.

@chriseth
Copy link
Contributor

In the end, this is a task for the mload resolver (eliminate the keccak) and the redundant store eliminator (eliminate the mstore as soon as the keccak is gone).

@hrkrshnn
Copy link
Member Author

Even the old optimizer isn't able to do this:

      0x00
      swap2
      dup3
      mstore
      0x20
      swap1
      swap2
      keccak256
      add
        /* "<stdin>":69:78  x[10] = 5 */
      sstore
        /* "<stdin>":28:82  function f() public returns (uint a) {... */

@chriseth
Copy link
Contributor

Maybe because it deems the result suboptimal?

@hrkrshnn
Copy link
Member Author

hrkrshnn commented Mar 1, 2021

I think the old optimizer is not able to do it because of an intermediate jump:

    tag_5:
        /* "<stdin>":57:63  uint a */
      0x00
        /* "<stdin>":77:78  5 */
      0x05
        /* "<stdin>":69:70  x */
      0x00
        /* "<stdin>":71:73  10 */
      0x0a
        /* "<stdin>":69:74  x[10] */
      dup2
      sload
      dup2
      lt
      tag_9
      jumpi
      mstore(0x00, 0x4e487b7100000000000000000000000000000000000000000000000000000000)
      mstore(0x04, 0x32)
      revert(0x00, 0x24)
    tag_9:
      0x00
      swap2
      dup3
      mstore
      0x20
      swap1
      swap2
      keccak256
      add
        /* "<stdin>":69:78  x[10] = 5 */
      sstore
        /* "<stdin>":28:82  function f() public returns (uint a) {... */
      swap1
      jump	// out

@chriseth
Copy link
Contributor

chriseth commented Mar 1, 2021

Ah, that is probably true!

@NunoFilipeSantos NunoFilipeSantos added the stale The issue/PR was marked as stale because it has been open for too long. label Nov 25, 2022
@github-actions
Copy link

github-actions bot commented Feb 6, 2023

Hi everyone! This issue has been closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Feb 6, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 6, 2023
@frangio
Copy link
Contributor

frangio commented Feb 6, 2023

I believe this should remain open, it's a concrete optimization that should be implemented.

@cameel
Copy link
Member

cameel commented Feb 6, 2023

We have a lot of these optimizations in the backlog and we're starting to think that having an issue for each one is not the best way to keep track of them. May don't have enough detail to just hand the task to someone for implementation. Some of them are even mutually exclusive. It's just a messy wishlist right now and a lot of it may end up never being implemented.

@ekpyron suggested creating some document to track them instead. Maybe we should have issues only for more concrete stuff we actually intend to implement in the near future.

But since you're clearly interested in this one, I'll reopen it for the time being just so that it remains more visible.

@cameel cameel reopened this Feb 6, 2023
@cameel cameel removed closed due inactivity The issue/PR was automatically closed due to inactivity. stale The issue/PR was marked as stale because it has been open for too long. labels Feb 6, 2023
@frangio
Copy link
Contributor

frangio commented Feb 6, 2023

I don't have a special interest in this one, but I definitely have an interest in optimizations in general. I commented because I felt that if this was closed it would get lost. I support whatever you consider the most effective way to keep track of optimization opportunities.

@github-actions
Copy link

github-actions bot commented May 8, 2023

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label May 8, 2023
@github-actions
Copy link

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label May 15, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. optimizer stale The issue/PR was marked as stale because it has been open for too long.
Projects
No open projects
Yul
  
Inbox
Development

No branches or pull requests

5 participants