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.

Sign up for GitHub

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

Jump to bottom

Refactor update script #1596

Merged
merged 3 commits into from
Nov 30, 2024
Merged

Refactor update script #1596

merged 3 commits into from
Nov 30, 2024

Conversation

goldsteinn
Copy link
Collaborator

This primary purpose is to support machine specific compile times and move more of the codes to a common python script.

Tested the functionality (without pushing) locally.

This does not come with the ci scripts:

New functionalities:

Update baseline compile time:

python3 scripts/update.py --baseline --bench bench --comptime --no-diff

Get compile time for a patch + create diffs:

python3 scripts/update.py --bench bench --comptime

Update baseline stats + diffs:

python3 scripts/update.py --baseline --bench bench --stats

Update stats for a patch + create diffs:

python3 scripts/update.py --bench bench --stats

@goldsteinn goldsteinn requested a review from dtcxzyw November 1, 2024 19:19
@goldsteinn
Copy link
Collaborator Author

So regarding how to integrate this, we have two options. 1) create support for users just requesting compile time in which case the workflow would look like:

python3 scripts/update.py --baseline --bench bench --comptime --no-diff
# Apply user patches (basically setup_pre_commit_patch.sh)
python3 scripts/update.py --bench bench --comptime --out pr-comment.md

Or would could just add those two steps to the default case.

Note the:

python3 scripts/update.py --baseline --bench bench --comptime --no-diff

Will only need to run-once per update of llvm-project.

dtcxzyw
dtcxzyw reviewed Nov 2, 2024
View reviewed changes
scripts/comptime_align.py Outdated Show resolved Hide resolved
scripts/comptime_diff.py Outdated Show resolved Hide resolved
scripts/comptime_align.py Outdated Show resolved Hide resolved
scripts/stats_diff.py Outdated Show resolved Hide resolved
scripts/update.py Show resolved Hide resolved
scripts/update.py Outdated Show resolved Hide resolved
scripts/update_optimized.sh Show resolved Hide resolved
@goldsteinn
Copy link
Collaborator Author

NB: Didn't test V2.

dtcxzyw
dtcxzyw reviewed Nov 5, 2024
View reviewed changes
Copy link
Owner

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

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

Please make sure that the llvm submodule version is the same with the llvm version in the base branch. You can run git submodule update after git pull :)

scripts/update_optimized.sh Outdated Show resolved Hide resolved
scripts/update.py Outdated Show resolved Hide resolved
@goldsteinn
Refactor update script
c90f9bf 
This primary purpose is to support machine specific compile times and
move more of the codes to a common python script.

Tested the functionality (without pushing) locally.

This does not come with the ci scripts:

New functionalities:

Update baseline compile time:
```
python3 scripts/update.py --baseline --bench bench --comptime --no-diff
```

Get compile time for a patch + create diffs:
```
python3 scripts/update.py --bench bench --comptime
```

Update baseline stats + diffs:
```
python3 scripts/update.py --baseline --bench bench --stats
```

Update stats for a patch + create diffs:
```
python3 scripts/update.py --bench bench --stats
```
@goldsteinn
Fixups
c66c9b6
@goldsteinn
Drop Debug Script/Issue on Baseline
6d04fa3
@goldsteinn goldsteinn force-pushed the goldsteinn/update-scripts branch from f8accf2 to 6d04fa3 Compare November 5, 2024 15:58
dtcxzyw
dtcxzyw approved these changes Nov 7, 2024
View reviewed changes
Copy link
Owner

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

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

LG. I will land this patch at my convenience.

scripts/update.py
"-vectorize-loops=false", "-vectorize-slp=false", input_file, "-S"
]
tmp_output = output_file + ".bench_tmp.ll"
cmd += ["-o", tmp_output]
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
cmd += ["-o", tmp_output]
cmd += ["-o", tmp_output] if COMPTIME_OUT is None else ['--disable-output']

It would make the instruction count more stable.

@dtcxzyw
Copy link
Owner

dtcxzyw commented Nov 27, 2024

I will land this patch on Friday.

@goldsteinn
Copy link
Collaborator Author

I will land this patch on Friday.

Okay, any plans for the benchmarks support scripts?

@dtcxzyw
Copy link
Owner

dtcxzyw commented Nov 27, 2024

I will land this patch on Friday.

Okay, any plans for the benchmarks support scripts?

Do you mean the scripts for automatic version bumping of benchmarks?

@goldsteinn
Copy link
Collaborator Author

I will land this patch on Friday.

Okay, any plans for the benchmarks support scripts?

Do you mean the scripts for automatic version bumping of benchmarks?

Err, did you see my above comment, but I mean integration into the ci scripts

@dtcxzyw
Copy link
Owner

dtcxzyw commented Nov 27, 2024

I mean integration into the ci scripts

Oh, I got it. I will implement the first option after this patch is verified on the upstream.

@dtcxzyw dtcxzyw merged commit fa7242e into dtcxzyw:main Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtcxzyw dtcxzyw dtcxzyw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@goldsteinn @dtcxzyw