Skip to content

Integrate summaries of opcode automatically into the evm semantics #2710

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

Closed
wants to merge 131 commits into from

Conversation

Stevengre
Copy link
Contributor

@Stevengre Stevengre commented Feb 27, 2025

This PR is based on #2676, integrating all the summaries into the evm semantics. Specifically, it includes the following changes:

  1. Enhance summarize command with opcode-specific and clear options.
  2. Modify the function to generate correct summary files that can be used directly.
  3. Provide new kdist build targets using these summary rules.
  4. Modify the .gitignore to integrate the summary files by default.

Copy link
Contributor

@tothtamas28 tothtamas28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a description for the PR, and make sure tests pass.

@Stevengre Stevengre self-assigned this Mar 4, 2025
@Stevengre Stevengre force-pushed the jh/summary-integration branch from f6646f4 to 82c51a7 Compare March 7, 2025 13:12
@Stevengre
Copy link
Contributor Author

Change files:

  • summarizer.py: Although there are 113 change files, the key file to this PR is summarizer.py, where I modify the summarize function to print kompileable K files.
  • summaries/: All the K files in summaries are generated automatically through the summarizer.py and tested by the kidst build.
  • driver.md & edsl.md: I provided two new modules for the kidst to target.
  • __main.py & cli.py: Here, I made a better option for the summarize command.

@Stevengre
Copy link
Contributor Author

Stevengre commented Mar 7, 2025

Should I provide tests to test the summary rules? Just like what we've done for the exising one, showing that the summary rules do the same as the previous ones. Or should we verify if the rules are correct or not?

Should we provide these tests inside of the CI? Should delete the previous CI test for the summarzation after we've provided the one for testing/verifying the summarization result.

@Stevengre Stevengre requested a review from ehildenb March 7, 2025 14:08
@Stevengre Stevengre marked this pull request as ready for review March 7, 2025 14:08
@Stevengre
Copy link
Contributor Author

Or maybe I can verify the correctness of these summary rules and migrate all the tests through the normal semantics to the summary ones?

Comment on lines +19 to +24
module EDSL-SUMMARY
imports EDSL-PURE
imports SUMMARY
endmodule

module EDSL-SUMMARIZE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between these two modules?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUMMARY is to use the summary rules.
SUMMARIZE is used to generate the summary rules.

@@ -643,13 +698,12 @@ def batch_summarize(num_processes: int = 4) -> None:
num_processes: Number of parallel processes to use. Defaults to 4.
"""

opcodes_to_process = OPCODES.keys()
OPCODES.keys()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
OPCODES.keys()

Comment on lines 609 to 610
def _remove_inf_gas(res_line: str) -> str:
return re.sub(r'#gas (\([^)]*\))', r'\1', res_line)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to do these transformations directly on the KRule produced by summarization?

Copy link
Contributor Author

@Stevengre Stevengre Mar 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is possible. But I don't know if it is possible for all. I'll give it a try.

@Stevengre Stevengre force-pushed the jh/summary-integration branch from ce061ca to 12fe54e Compare March 12, 2025 17:07
- Remove unnecessary account constraints and DotAccountVar references
- Modify summarizer to handle complex regex replacements for cleaner spec files
- Update opcode symbol parsing to handle parentheses in opcode names
- Remove redundant requires and andBool conditions in summary specs
- Rename an undefined opcode summary file to fix import syntax
…constraints

- Modify summarizer to comment out complex regex replacements
- Update summary files to use simpler account representation
- Remove parenthesized account and DotAccountVar references
- Restore _use_legal_remainder function with commented-out complex logic
- Update summary files for numerous EVM opcodes
- Modify basic block rules in summary specification files
- Systematically process and generate summary files for various EVM instructions
- Add function to replace LHS function with assignment in summary-balance specs
- Modify account ID representation in balance summary files
- Update regex transformation to simplify account ID constraints
- Reduce summarization test timeout from 360 to 150 seconds
- Remove duplicate method in KEVM class
- Update poetry.lock dependencies and extras
- Modify batch summarization processing logic
- Clean up regex transformations in summary specification generation
- Modify Poetry lock file to add platform-specific extras and dependency constraints
- Update Poetry version from 2.0.1 to 2.1.1
- Add conditional dependencies for different Python implementations and versions
- Refine package extras with more precise platform and version requirements
- Upgrade Hypothesis from 6.125.2 to 6.127.9
- Update setuptools from 75.8.0 to 75.8.2
- Add watchdog dependency to Hypothesis extras
- Refresh package hashes and version constraints
- Modify account ID replacement logic in summarizer
- Remove commented-out explanatory notes
- Streamline regex pattern for summary-balance specifications
- Reformat target argument definition for better readability
- Maintain existing target choices for KEVM CLI
@Stevengre Stevengre force-pushed the jh/summary-integration branch from 30a0dcc to 3423f02 Compare March 14, 2025 18:00
- Reduced timeout for the 'Summarization' test suite from 150 to 30 seconds.
- Updated dependencies: filelock to version 3.18.0, hypothesis to version 6.129.1, and setuptools to version 76.0.0 in poetry.lock.
…from summarizer and summary.k files to address potential issues with log2 calculations.
…rences

- Enhanced error handling in the interpreter to raise a RuntimeError with detailed information if the interpreter fails or returns empty output.
- Restored references to 'summary-exp-2-spec.k' and 'SUMMARY-EXP-2-SPEC' in the summarizer and summary.k files to ensure proper functionality after previous removals.
@Stevengre
Copy link
Contributor Author

Stevengre commented Mar 17, 2025

Performances

pure is kdist target without EVM-OPTIMIZATIONS; summary is kdist target with SUMMARY.

command kdist real user sys
make test-conformance llvm-pure 5m37.384s 12m5.376s 1m42.848s
make test-conformance llvm 5m15.323s 11m19.932s 1m38.279s
make test-conformance llvm-summary 5m3.396 11m25.853s 1m37.160s
make test-prove-rules haskell-pure
make test-prove-rules haskell
make test-prove-rules haskell-summary
make test-prove-functional haskell-pure
make test-prove-functional haskell
make test-prove-functional haskell-summary
make test-prove-dss haskell-pure
make test-prove-dss haskell
make test-prove-dss haskell-summary
  • For concrete execution,
    • The summary rules perform better than the semantics without EVM-OPTIMIZATIONS, but worse than the one with EVM-OPTIMIZATIONS. It shows that the summary rules do summarize the rules for the opcode like EVM-OPTIMIZATIONS, but the summarization is not good as EVM-OPTIMIZATIONS. For example, EVM-OPTIMIZATIONS only use one rule for ADD, however, SUMMARY-ADD-2-SPEC takes two. What makes it worse should be the summary rules for dup, swap and log, because they consumes mutable wordstack which ends up with more modules for them under current summarization implementation. Moreover, some opcodes don't support use-gas summarization, which cannot be used for concrete execution.
    • Time reduction is less than what I expected (10+ steps into 1 step). It might due to the programs under tested are small and it costs more time to do something else rather than run opcodes. I'm not sure for this analysis.
    • Here is another result for summary rules (real 3m34.911s; user 11m15.463s; sys 1m37.799s) (real 3m35.102s; user 11m12.657s; sys 1m35.723s). Therefore, summary rules might not be worse than optimization rules. But llvm (real 3m32.791s; user 11m3.709s; sys 1m36.085s)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants