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

Upgrade llvm 18 zig 13 #6921

Merged
merged 100 commits into from
Dec 13, 2024
Merged

Upgrade llvm 18 zig 13 #6921

merged 100 commits into from
Dec 13, 2024

Conversation

lukewilliamboswell
Copy link
Collaborator

No description provided.

flake.nix Outdated Show resolved Hide resolved
bhansconnect added a commit that referenced this pull request Dec 12, 2024
To get #6921 working, we need the benchmark script to pull glue.zig correctly on main.
As such, we need to load it into a lib dir such that it is found in the correct relative path.
This hopefully will get the benchmarking working on the other PR.

Also, switch over to `lib` dir first cause that is what is seen in the wide.
bhansconnect added a commit that referenced this pull request Dec 12, 2024
To get #6921 working, we need the benchmark script to pull glue.zig correctly on main.
As such, we need to load it into a lib dir such that it is found in the correct relative path.
This hopefully will get the benchmarking working on the other PR.

Also, switch over to `lib` dir first cause that is what is seen in the wide.
bhansconnect added a commit that referenced this pull request Dec 12, 2024
To get #6921 working, we need the benchmark script to pull glue.zig correctly on main.
As such, we need to load it into a lib dir such that it is found in the correct relative path.
This hopefully will get the benchmarking working on the other PR.

Also, switch over to `lib` dir first cause that is what is seen in the wide.
Comment on lines +37 to +38
// TODO: double check how much time is spent verifying here and below.
// For real compilation, we may not want to pay the cost.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@bhansconnect do you think we need to do this before merging?

Copy link
Contributor

Choose a reason for hiding this comment

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

no

Copy link
Contributor

Choose a reason for hiding this comment

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

So I just did some quick test. Tested with a debug build of helloworld (cause that is the case where a little extra cost hurts the most).

The check here costs about 15ms. The second check is free. The total build time before linking is about 100ms. Linking without the surgical linker is about 150ms. My gut feeling is just to leave this for now with the todo and circle back later. Long term, we probably want to remove at least the first check here (when we do make it dump the llvm ir if the passes fail to run). But for now, verifying early seems like a good idea.

@lukewilliamboswell lukewilliamboswell merged commit a0de21d into main Dec 13, 2024
17 of 19 checks passed
@lukewilliamboswell lukewilliamboswell deleted the upgrade-llvm-zig branch December 13, 2024 04:11
Anton-4 added a commit that referenced this pull request Dec 13, 2024
examples/cli was removed in #6921
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.

5 participants