ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
23.37k stars 5.78k forks source link

When compiling Yul, optimizer does a lot even when disabled in settings #14470

Closed haltman-at closed 11 months ago

haltman-at commented 1 year ago

Description

When compiling Yul, setting enabled: false and even yul: false seems to leave on a lot of optimizations. Moreover, setting yul: false prohibits setting yulDetails; this means that to get code that isn't substantially optimized, you actually have to set yul: true so you can go into yulDetails and turn things off!

I'm a bit unsure how much this is a bug and how much this is intentional, but it didn't seem right to me (the part about having to set yul: true in particular strikes me as perverse), so I thought I'd file an issue. Thanks!

Environment

Steps to Reproduce

For what it's worth, the test contract I was trying this with was

object "YulTest" {
  code {
    let size := datasize("runtime")
    datacopy(0, dataoffset("runtime"), size)
    return(0, size)
  }
  object "runtime" {
    code {
      let a := 1
      let b := 2
      b := add(b,a)
      mstore(0, add(b, a))
      return(0, 0x20)
    }
  }
}

(Well, that, and quite a few variants on it.) The code above got substantially rewritten even with enabled: false and yul: false, as mentioned above! I had to set yul: true and go turn things off in yulDetails to get something that worked the way I wrote it above...

ekpyron commented 1 year ago

Related comment: https://github.com/ethereum/solidity/pull/14149#discussion_r1174026509

As mentioned via matrix, the "minimal optimizations" enabled in that PR are meant to solve compilability issues and are meant to conceptually be considered as part of regular code generation.

Can you elaborate on why you want to disable those "optimizations"? Are they causing any issues for you?

I'd be open to allowing a more intuitive and user-friendly way to disable those optimizations, but the idea was that the default settings should produce compiler output that's fine to consume for tooling and that the need to disable them should not arise. Can you elaborate on which substantial rewrites are causing issues for you, if any?

cameel commented 1 year ago

Yeah, it could be that it's actually interfering with what you're trying to do, but it's also possible that it just seems more "rewritten" than it actually is. The only optimizer step we run with yul: false is UnusedPruner. All it does is remove unreferenced functions, unreferenced variables and movable expression statements. So I wonder it's just the case of the diff looking horrible while the non-removed bits are actually intact.

And additional factor making it look worse might be that now the --ir output is always reformatted just like --ir-optimized is. It might even reorder functions compared to how it was before (though I'm not not 100% sure about that). That's the consequence of the Yul stack being always on. Even if no optimization is performed, the Yul code is still reparsed after code generation and only then printed to the output.

haltman-at commented 1 year ago

Hm, it's only supposed to be the unused pruner? Interesting. I'm definitely seeing more than that. (Although maybe everything else is the the result of the stack-reorderer? But that's still more than the unused pruner, and you still need to go into yulDetails, turning on yul: true, to turn that off.) I'm off work this week, so I don't have an example for you right now, but I'll get you one once I'm back next week!

cameel commented 1 year ago

By the way are you looking at the Yul or EVM asm output?

At the EVM assembly level you will definitely see more changes because it uses the new Yul->EVM transform and the mechanism for moving variables to memory. But this was kinda the point of this change.

Assawal commented 1 year ago

Assawal

On Mon, 7 Aug 2023, 11:33 pm Kamil Śliwak, @.***> wrote:

By the way are you looking at the Yul or EVM asm output?

At the EVM assembly level you will definitely see more changes because it uses the new EVM->Yul transform and the mechanism for moving variables to memory. But this was kinda the point of this change.

— Reply to this email directly, view it on GitHub https://github.com/ethereum/solidity/issues/14470#issuecomment-1668393564, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATTYHAUOE45BUTCC5CA22STXUEYF5ANCNFSM6AAAAAA3EZQK3A . You are receiving this because you are subscribed to this thread.Message ID: @.***>

ekpyron commented 1 year ago

My first guess would also be that the main difference you may be seeing may be the new Yul->EVM code transform, which will no longer generate code top-down statement-by-statement but tries to efficiently allocate stack slots and produce more optimal bytecode from Yul than the old code transform did - but that should still run with yul: true unless you set "stackAllocation": false in yulDetails. Ideally, we'd like to deprecate and remove the old code transform (and thereby the stackAllocation setting) entirely - so if this is causing issues for you, that'd be important for us to know.

haltman-at commented 1 year ago

Hm, interesting. Yes to be clear, I was only looking at the final result (EVM directly). Or rather -- I wasn't really looking at the bytecode directly. Rather, when I say "too much optimization", what I really mean is, variables aren't being placed in predictable places / aren't sticking in one place. It sounds like this may be a matter of the stackAllocation setting?

I mean of course this report is all coming from my point of view as the maintainer of Truffle Debugger. Basically I figured that, the debugger does fine detecting Yul variables when used inside Solidity assembly with optimization turned off; how come when I compile straight Yul, I seemingly get more optimization that completely throws it off? I guess this is because compiling straight Yul is using the IR pipeline that uses things like the stackAllocation setting?

This is why I wasn't really sure there's a bug here. After all -- it's not a bug if Solidity, or Yul, or anything, places its variables in ways we can't predict or locate. It's our job to locate the variables, not your job to make them locatable! So if all this is intentional, there's no bug, just one more thing we have to figure out how to deal with. But, due to the differences I was seeing between Yul inside Solidity assembly and Yul straight, I thought there might be a chance that this was actually a bug, and that it wasn't supposed to be doing this much with optimization turned off. Sounds like probably that's not the case, though?

I should probably come back later and post an actual concrete example, but yeah, hopefully this clarifies what I was talking about.

ekpyron commented 1 year ago

When we compile inline assembly in legacy (non-via-IR) code generation, we still use the older transformation from Yul to EVM bytecode (the reason for that is that in that pipeline the compiler itself relies on the more predictable allocation of stack slots for tying the assembly/Yul snippets to the opcodes directly generated from the surrounding Solidity code) - so that explains that reconstructing stack locations is simpler in that case.

Conversely, when compiling Yul or when compiling Solidity using the new via-IR pipeline, we use more elaborate and global mechanisms for assigning stack slots (that used to only happen with the optimizer enabled, but after https://github.com/ethereum/solidity/pull/14149 it's also the default behaviour without optimizer - I mentioned that change in solc-tooling here https://matrix.to/#/!nFrzmgewkRourOtwgQ:matrix.org/$8b62xhTVSM_K7tnGnLojkD1h55Mz4EiyWNrCm8dGzfE?via=matrix.org&via=t2bot.io&via=trufflesuite.com and was actually surprised that nobody reported any issues, but in retrospect I should have tried harder to get more attention to it). For that case I'd actually even advice against trying to reconstruct how we assign stack locations in the debugger - the process is complex and based on not readily-reconstructible heuristics; local changes to the Yul code can have global effects on the stack layout, etc.

That's one of the main reasons for ethdebug - we know the stack locations during compilation and would ideally just output them for you. It's a bit unfortunate that there will be some compiler versions until we actually output that information for you to bridge.

mehtavishwa30 commented 11 months ago

Thanks for reporting this issue. We will be closing this for now and expect Ethdebug to address this issue eventually.