[Yul] Add structural simplifier.

[ekpyron]

Closes #5290.

[codecov]

Codecov Report

Merging #5584 into develop will increase coverage by <.01%.
The diff coverage is 89.87%.

@@             Coverage Diff             @@
##           develop    #5584      +/-   ##
===========================================
+ Coverage    87.98%   87.99%   +<.01%     
===========================================
  Files          332      334       +2     
  Lines        32108    32183      +75     
  Branches      3855     3863       +8     
===========================================
+ Hits         28250    28319      +69     
- Misses        2551     2554       +3     
- Partials      1307     1310       +3

Flag	Coverage Δ
#all	87.99% <89.87%> (ø)	⬆️
#syntax	28.71% <0%> (-0.07%)	⬇️

[chriseth]

check

[chriseth]

I think it makes sense to run this before the unused pruner, because it could remove some uses of some variables.

[ekpyron]

I moved it to after each run of the expression simplifier now, that's probably a good spot.

[ekpyron]

Oh - I hadn't noticed that the appveyor failure is in fact relevant: there are different variable names generated, so may be some nondeterminism

[ekpyron]

@chriseth We had an indication of non-determinism in the appveyor test run before (different suffices in some test case) - so we should at least wait for the appveyor test to succeed.

[ekpyron]

Still there: for reference:

C:/projects/solidity/test/boostTest.cpp(114): error: in "yulOptimizerTests/fullSuite/medium": Test expectation mismatch.
Expected result:
  fullSuite
  {
      {
          let _1 := 0x20
          let allocate__17 := 0x40
          mstore(allocate__17, add(mload(allocate__17), _1))
          let allocate_p_22_39 := mload(allocate__17)
          mstore(allocate__17, add(allocate_p_22_39, allocate__17))
          mstore(add(allocate_p_22_39, 96), 2)
          mstore(allocate__17, _1)
      }
  }
Obtained result:
  fullSuite
  {
      {
          let _1 := 0x20
          let allocate__19 := 0x40
          mstore(allocate__19, add(mload(allocate__19), _1))
          let allocate_p_24_41 := mload(allocate__19)
          mstore(allocate__19, add(allocate_p_24_41, allocate__19))
          mstore(add(allocate_p_24_41, 96), 2)
          mstore(allocate__19, _1)
      }
  }
***
*** 1 failure is detected in the test module "SolidityTests"

So we have to look into this before merging.

It's a "harmless" difference in a sense, since it's just an alphabetic variant, but still.

[chriseth]

Nice!

[chriseth]

That's not funny....
Also I don't see how this PR could have affected it, which is much less funny...

Dismissing to prevent accidental merge before clarifying the non-determinism.

[ekpyron]

@chriseth Yes, so far I don't see the cause and it may be unrelated to this PR and just "randomly" triggered here, but "at least" the difference seems to happen deterministically for this PR now.

[chriseth]

Fingers crossed.

Still needs to be squashed.

[ekpyron]

Tests just passed. I'm squashing.

[ekpyron]

Squashed.