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

Bytecode report presets #14350

Merged
merged 1 commit into from
Jun 23, 2023
Merged

Bytecode report presets #14350

merged 1 commit into from
Jun 23, 2023

Conversation

cameel
Copy link
Member

@cameel cameel commented Jun 23, 2023

Prerequisite for #13583.

Currently the bytecode report generation is hard-coded to output results of unoptimized and then optimized compilation. This PR organizes these these settings into distinct presets and adds command-line flags for selecting an arbitrary subset of those presets.

In subsequent PRs I'll use this mechanism to add via IR presets. This PR is a pure refactor that does not change behavior and preserves the default behavior of the scripts (since they're also used for PR checks in solc-bin).

The mechanism will also later be used to generate reports for all presets in parallel in CI.

Note that there are many whitespace-only changes in the JS script. I recommend turning on the mode that ignores whitespace changes.

matheusaaguiar
matheusaaguiar previously approved these changes Jun 23, 2023
Copy link
Collaborator

@matheusaaguiar matheusaaguiar left a comment

Choose a reason for hiding this comment

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

Just two minor questions which I don't think prevent this from being merged.

scripts/bytecodecompare/prepare_report.js Outdated Show resolved Hide resolved
scripts/bytecodecompare/prepare_report.js Show resolved Hide resolved
@cameel cameel merged commit aca4c86 into develop Jun 23, 2023
1 check passed
@cameel cameel deleted the bytecode-report-presets branch June 23, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants