[EVM] Expand SELECT into arithmetic operations

[PavelKopyl]

Code Review Checklist

Purpose

Ticket Number

Requirements

• Have the requirements been met?
• Have stakeholder(s) approved the change?

Implementation

• Does this code change accomplish what it is supposed to do?
• Can this solution be simplified?
• Does this change add unwanted compile-time or run-time dependencies?
• Could an additional framework, API, library, or service improve the solution?
• Could we reuse part of LLVM instead of implementing the patch or a part of it?
• Is the code at the right abstraction level?
• Is the code modular enough?
• Can a better solution be found in terms of maintainability, readability, performance, or security?
• Does similar functionality already exist in the codebase? If yes, why isn’t it reused?
• Are there any best practices, design patterns or language-specific patterns that could substantially improve this code?

Logic Errors and Bugs

• Can you think of any use case in which the
  code does not behave as intended?
• Can you think of any inputs or external events
  that could break the code?

Error Handling and Logging

• Is error handling done the correct way?
• Should any logging or debugging information
  be added or removed?
• Are error messages user-friendly?
• Are there enough log events and are they
  written in a way that allows for easy
  debugging?

Maintainability

• Is the code easy to read?
• Is the code not repeated (DRY Principle)?
• Is the code method/class not too long?

Dependencies

• Were updates to documentation, configuration, or readme files made as required by this change?
• Are there any potential impacts on other parts of the system or backward compatibility?

Security

• Does the code introduce any security vulnerabilities?

Performance

• Do you think this code change decreases
  system performance?
• Do you see any potential to improve the
  performance of the code significantly?

Testing and Testability

• Is the code testable?
• Have automated tests been added, or have related ones been updated to cover the change?
  • For changes to mutable state
• Do tests reasonably cover the code change (unit/integration/system tests)?
  • Line Coverage
  • Region Coverage
  • Branch Coverage
• Are there some test cases, input or edge cases
  that should be tested in addition?

Readability

• Is the code easy to understand?
• Which parts were confusing to you and why?
• Can the readability of the code be improved by
  smaller methods?
• Can the readability of the code be improved by
  different function, method or variable names?
• Is the code located in the right
  file/folder/package?
• Do you think certain methods should be
  restructured to have a more intuitive control
  flow?
• Is the data flow understandable?
• Are there redundant or outdated comments?
• Could some comments convey the message
  better?
• Would more comments make the code more
  understandable?
• Could some comments be removed by making the code itself more readable?
• Is there any commented-out code?
• Have you run a spelling and grammar checker?

Documentation

• Is there sufficient documentation?
• Is the ReadMe.md file up to date?

Best Practices

• Follow Single Responsibility principle?
• Are different errors handled correctly?
• Are errors and warnings logged?
• Magic values avoided?
• No unnecessary comments?
• Minimal nesting used?

Experts' Opinion

• Do you think a specific expert, like a security
  expert or a usability expert, should look over
  the code before it can be accepted?
• Will this code change impact different teams, and should they review the change as well?

[github-actions]

Results for: evm ir-llvm EVMInterpreter
╔═╡ Size (-%) ╞═══════════════════════╡ All E +M3B3 ╞═╗
║ Best                                          3.354 ║
║ Worst                                        -3.237 ║
║ Total                                         0.000 ║
╠═╡ Cycles (-%) ╞═════════════════════╡ All E +M3B3 ╞═╣
║ Best                                          3.810 ║
║ Worst                                       -43.704 ║
║ Total                                        -0.009 ║
╠═╡ Ergs (-%) ╞═══════════════════════╡ All E +M3B3 ╞═╣
║ Best                                          2.317 ║
║ Worst                                       -19.054 ║
║ Total                                        -0.007 ║
╠══╡ Gas (-%) ╞═══════════════════════╡ All E +M3B3 ╞═╣
║ Best                                          3.444 ║
║ Worst                                       -40.315 ║
║ Total                                         0.000 ║
╚═════════════════════════════════════════════════════╝

╔═╡ Size (-%) ╞═══════════════════════╡ All E +MzB3 ╞═╗
║ Best                                          3.559 ║
║ Worst                                        -3.321 ║
║ Total                                        -0.008 ║
╠═╡ Cycles (-%) ╞═════════════════════╡ All E +MzB3 ╞═╣
║ Best                                          2.552 ║
║ Worst                                       -43.410 ║
║ Total                                        -0.011 ║
╠═╡ Ergs (-%) ╞═══════════════════════╡ All E +MzB3 ╞═╣
║ Best                                          2.559 ║
║ Worst                                       -19.065 ║
║ Total                                        -0.009 ║
╠══╡ Gas (-%) ╞═══════════════════════╡ All E +MzB3 ╞═╣
║ Best                                          3.631 ║
║ Worst                                       -39.545 ║
║ Total                                         0.000 ║
╚═════════════════════════════════════════════════════╝

╔═╡ Size (-%) ╞═══════════════════════╡ All Y +M3B3 ╞═╗
║ Best                                          2.776 ║
║ Worst                                        -3.420 ║
║ Total                                        -0.014 ║
╠═╡ Cycles (-%) ╞═════════════════════╡ All Y +M3B3 ╞═╣
║ Best                                          7.055 ║
║ Worst                                       -72.380 ║
║ Total                                        -0.014 ║
╠═╡ Ergs (-%) ╞═══════════════════════╡ All Y +M3B3 ╞═╣
║ Best                                          2.936 ║
║ Worst                                       -44.761 ║
║ Total                                        -0.012 ║
╠══╡ Gas (-%) ╞═══════════════════════╡ All Y +M3B3 ╞═╣
║ Best                                          2.841 ║
║ Worst                                       -66.569 ║
║ Total                                        -0.000 ║
╚═════════════════════════════════════════════════════╝

╔═╡ Size (-%) ╞═══════════════════════╡ All Y +MzB3 ╞═╗
║ Best                                          1.524 ║
║ Worst                                        -4.567 ║
║ Total                                        -0.033 ║
╠═╡ Cycles (-%) ╞═════════════════════╡ All Y +MzB3 ╞═╣
║ Best                                          1.892 ║
║ Worst                                       -62.953 ║
║ Total                                        -0.021 ║
╠═╡ Ergs (-%) ╞═══════════════════════╡ All Y +MzB3 ╞═╣
║ Best                                          2.792 ║
║ Worst                                       -41.344 ║
║ Total                                        -0.017 ║
╠══╡ Gas (-%) ╞═══════════════════════╡ All Y +MzB3 ╞═╣
║ Best                                          2.444 ║
║ Worst                                       -56.861 ║
║ Total                                        -0.000 ║
╚═════════════════════════════════════════════════════╝

╔═╡ Size (-%) ╞═════════════════╡ Real life E +M3B3 ╞═╗
║ Best                                          0.445 ║
║ Worst                                        -3.237 ║
║ Total                                         0.059 ║
╠═╡ Cycles (-%) ╞═══════════════╡ Real life E +M3B3 ╞═╣
║ Best                                          0.038 ║
║ Worst                                       -43.704 ║
║ Total                                        -1.139 ║
╠═╡ Ergs (-%) ╞═════════════════╡ Real life E +M3B3 ╞═╣
║ Best                                          0.013 ║
║ Worst                                       -19.054 ║
║ Total                                        -0.308 ║
╠══╡ Gas (-%) ╞═════════════════╡ Real life E +M3B3 ╞═╣
║ Best                                          0.440 ║
║ Worst                                       -40.315 ║
║ Total                                         0.114 ║
╚═════════════════════════════════════════════════════╝

╔═╡ Size (-%) ╞═════════════════╡ Real life E +MzB3 ╞═╗
║ Best                                          0.466 ║
║ Worst                                        -3.321 ║
║ Total                                         0.023 ║
╠═╡ Cycles (-%) ╞═══════════════╡ Real life E +MzB3 ╞═╣
║ Best                                          0.347 ║
║ Worst                                       -43.410 ║
║ Total                                        -1.078 ║
╠═╡ Ergs (-%) ╞═════════════════╡ Real life E +MzB3 ╞═╣
║ Best                                          0.408 ║
║ Worst                                       -19.065 ║
║ Total                                        -0.265 ║
╠══╡ Gas (-%) ╞═════════════════╡ Real life E +MzB3 ╞═╣
║ Best                                          0.460 ║
║ Worst                                       -39.545 ║
║ Total                                         0.103 ║
╚═════════════════════════════════════════════════════╝

╔═╡ Size (-%) ╞═════════════════╡ Real life Y +M3B3 ╞═╗
║ Best                                          0.331 ║
║ Worst                                        -3.420 ║
║ Total                                         0.052 ║
╠═╡ Cycles (-%) ╞═══════════════╡ Real life Y +M3B3 ╞═╣
║ Best                                          0.281 ║
║ Worst                                       -44.395 ║
║ Total                                        -1.051 ║
╠═╡ Ergs (-%) ╞═════════════════╡ Real life Y +M3B3 ╞═╣
║ Best                                          0.301 ║
║ Worst                                       -19.164 ║
║ Total                                        -0.264 ║
╠══╡ Gas (-%) ╞═════════════════╡ Real life Y +M3B3 ╞═╣
║ Best                                          0.328 ║
║ Worst                                       -41.389 ║
║ Total                                         0.082 ║
╚═════════════════════════════════════════════════════╝

╔═╡ Size (-%) ╞═════════════════╡ Real life Y +MzB3 ╞═╗
║ Best                                          0.503 ║
║ Worst                                        -3.623 ║
║ Total                                         0.092 ║
╠═╡ Cycles (-%) ╞═══════════════╡ Real life Y +MzB3 ╞═╣
║ Best                                          0.338 ║
║ Worst                                       -44.207 ║
║ Total                                        -0.901 ║
╠═╡ Ergs (-%) ╞═════════════════╡ Real life Y +MzB3 ╞═╣
║ Best                                          0.426 ║
║ Worst                                       -19.154 ║
║ Total                                        -0.222 ║
╠══╡ Gas (-%) ╞═════════════════╡ Real life Y +MzB3 ╞═╣
║ Best                                          0.502 ║
║ Worst                                       -40.578 ║
║ Total                                         0.142 ║
╚═════════════════════════════════════════════════════╝

[PavelKopyl]

Expanding a select instruction with non-constant operands into a sequence of arithmetic operations may lead to significant performance regressions. In the linearized form, both the "true" and "false" values must always be computed, whereas when lowered to CFG form, each value is only evaluated if it is actually needed. This effect can be observed in tests/solidity/complex/defi/UniswapV4/test.json, where we have the following LLVM IR:

shift_left_join48:                                ; preds = %entry              
  %1 = trunc i256 %xor_result to i1                                                                                  
  %xor_result69 = select i1 %1, i256 340265354078544963557816517032075149313, i256 340282366920938463463374607431768211456
  %2 = and i256 %xor_result, 2                                                                                             
  %comparison_result84 = icmp eq i256 %2, 0                                                         
  %multiplication_result89 = mul nuw i256 %xor_result69, 340248342086729790484326174814286782778
  %shift_right_non_overflow_result = lshr i256 %multiplication_result89, 128                                          
  %stack_var_004.0 = select i1 %comparison_result84, i256 %xor_result69, i256 %shift_right_non_overflow_result
  %3 = and i256 %xor_result, 4               
  %comparison_result108 = icmp eq i256 %3, 0                                                         
  %multiplication_result113 = mul nuw i256 %stack_var_004.0, 340214320654664324051920982716015181260
  %shift_right_non_overflow_result123 = lshr i256 %multiplication_result113, 128                                       
  %stack_var_004.1 = select i1 %comparison_result108, i256 %stack_var_004.0, i256 %shift_right_non_overflow_result123
  %4 = and i256 %xor_result, 8                                                                          
  %comparison_result140 = icmp eq i256 %4, 0                                                         
  %multiplication_result145 = mul nuw i256 %stack_var_004.1, 340146287995602323631171512101879684304
  %shift_right_non_overflow_result155 = lshr i256 %multiplication_result145, 128                                       
  %stack_var_004.2 = select i1 %comparison_result140, i256 %stack_var_004.1, i256 %shift_right_non_overflow_result155
  %5 = and i256 %xor_result, 16              
  %comparison_result172 = icmp eq i256 %5, 0                                                             
  %multiplication_result177 = mul nuw i256 %stack_var_004.2, 340010263488231146823593991679159461444
  %shift_right_non_overflow_result187 = lshr i256 %multiplication_result177, 128                                       
  %stack_var_004.3 = select i1 %comparison_result172, i256 %stack_var_004.2, i256 %shift_right_non_overflow_result187
  %6 = and i256 %xor_result, 32                                                              
  %comparison_result204 = icmp eq i256 %6, 0                                                            
  %multiplication_result209 = mul nuw i256 %stack_var_004.3, 339738377640345403697157401104375502016
  %shift_right_non_overflow_result219 = lshr i256 %multiplication_result209, 128                                       
  %stack_var_004.4 = select i1 %comparison_result204, i256 %stack_var_004.3, i256   %shift_right_non_overflow_result219
  %7 = and i256 %xor_result, 64                                                      
  %comparison_result236 = icmp eq i256 %7, 0                                                            
  %multiplication_result241 = mul nuw i256 %stack_var_004.4, 339195258003219555707034227454543997025
  %shift_right_non_overflow_result251 = lshr i256 %multiplication_result241, 128                                       
  %stack_var_004.5 = select i1 %comparison_result236, i256 %stack_var_004.4, i256      %shift_right_non_overflow_result251
...

In the linearized code, we have to always calculate fragments like:

 %multiplication_result89 = mul nuw i256 %xor_result69, 340248342086729790484326174814286782778
 %shift_right_non_overflow_result = lshr i256 %multiplication_result89, 128                                   

Whereas in the CFG form they are not executed at all on some inputs.

[akiramenai]

I see, bad idea - it seems.
Run forge benchmarks just to double-check

[PavelKopyl]

I see, bad idea - it seems. Run forge benchmarks just to double-check

I guess it's useful in some cases, but we need perform some analysis where this reasonable where it's not, bot not expand it unconditionally. Let's create an issue for this?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 30.06%. Comparing base (d719238) to head (c42af9e).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #877      +/-   ##
==========================================
- Coverage   30.07%   30.06%   -0.01%     
==========================================
  Files        2441     2441              
  Lines      807942   807949       +7     
  Branches   176474   176475       +1     
==========================================
- Hits       242962   242946      -16     
- Misses     522806   522846      +40     
+ Partials    42174    42157      -17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

📊 Excel Report Available

Benchmarks measured for:

• solx candidate version: solx, LLVM-based Solidity compiler for the EVM v0.1.1, LLVM revision: v1.0.2, LLVM build: c42af9e4968010c8b2bb89887897685a5fad027b
• solx ToT version: solx, LLVM-based Solidity compiler for the EVM v0.1.1, LLVM revision: v1.0.2, LLVM build: d7192387c3fb7594be00716a6f56b9635d0ea4d2
• solx latest release version: solx, LLVM-based Solidity compiler for the EVM v0.1.1, LLVM revision: v1.0.2, LLVM build: b32c5ccab280fd4219e26aceb3f60f425e907498
• solc version: 0.8.30

➡️ Download Excel Report