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

[llvm] Adding missing global initializers when splitting a benchmark #775

Conversation

ChrisCummins
Copy link
Contributor

@ChrisCummins ChrisCummins commented Nov 7, 2022

  • Modify split_benchmark_by_function() so that it returns an extra "benchmark", which is a single module containing no executable instructions, but all global variable definitions. This is needed to make the result of merging the split benchmarks compilable.
  • Fig a buf in split_benchmark_by_function() whereby the definitions for functions marked with available_externally linkage were dropped.
  • Identify two passes, -strip and -strip-nondebug, which are unsafe to run before split+merge.
  • Add end-to-end tests that benchmark semantics are unchanged by splitting and merging.

Follow up to #772.

@ChrisCummins ChrisCummins added this to the v0.2.6 milestone Nov 7, 2022
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 7, 2022
@ChrisCummins ChrisCummins added this to In progress in CompilerGym roadmap via automation Nov 7, 2022
@ChrisCummins ChrisCummins added the LLVM LLVM-specific environment issue label Nov 7, 2022
@codecov-commenter
Copy link

codecov-commenter commented Nov 7, 2022

Codecov Report

Merging #775 (1faec6b) into development (768e4b7) will decrease coverage by 40.30%.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff                @@
##           development     #775       +/-   ##
================================================
- Coverage        89.01%   48.71%   -40.31%     
================================================
  Files              131      131               
  Lines             8065     8074        +9     
================================================
- Hits              7179     3933     -3246     
- Misses             886     4141     +3255     
Impacted Files Coverage Δ
compiler_gym/envs/llvm/llvm_benchmark.py 31.70% <0.00%> (-61.73%) ⬇️
...ompiler_gym/service/client_service_compiler_env.py 68.89% <0.00%> (-22.52%) ⬇️
compiler_gym/util/logging.py 0.00% <0.00%> (-100.00%) ⬇️
compiler_gym/wrappers/llvm.py 0.00% <0.00%> (-100.00%) ⬇️
compiler_gym/util/flags/seed.py 0.00% <0.00%> (-100.00%) ⬇️
compiler_gym/util/statistics.py 0.00% <0.00%> (-100.00%) ⬇️
compiler_gym/util/flags/nproc.py 0.00% <0.00%> (-100.00%) ⬇️
compiler_gym/util/permutation.py 0.00% <0.00%> (-100.00%) ⬇️
compiler_gym/wrappers/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
compiler_gym/util/capture_output.py 0.00% <0.00%> (-100.00%) ⬇️
... and 88 more

@ChrisCummins ChrisCummins force-pushed the feature/split-merge-integration-tests branch from a09aef7 to b4c48e2 Compare November 8, 2022 00:10
@ChrisCummins ChrisCummins force-pushed the feature/split-merge-integration-tests branch from b4c48e2 to bea46ba Compare December 13, 2022 02:36
If the function being extracted has `available_externally` linkage,
DCE will delete it, producing an empty file.
This adds support for custom benchmarks.
@ChrisCummins ChrisCummins force-pushed the feature/split-merge-integration-tests branch from bea46ba to b4a0da1 Compare December 13, 2022 06:35
@ChrisCummins ChrisCummins force-pushed the feature/split-merge-integration-tests branch from b4a0da1 to 1faec6b Compare December 13, 2022 06:38
@ChrisCummins ChrisCummins changed the title [llvm] Add integration test for split/merge benchmarks. [llvm] Adding missing global initializers when splitting a benchmark Dec 13, 2022
@ChrisCummins ChrisCummins merged commit 2f11598 into facebookresearch:development Mar 10, 2023
CompilerGym roadmap automation moved this from In progress to Done (not yet shipped) Mar 10, 2023
@ChrisCummins ChrisCummins deleted the feature/split-merge-integration-tests branch March 10, 2023 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. LLVM LLVM-specific environment issue
Projects
CompilerGym roadmap
Done (not yet shipped)
Development

Successfully merging this pull request may close these issues.

None yet

3 participants