-
-
Notifications
You must be signed in to change notification settings - Fork 319
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
Upgrade llvm 18 zig 13 #6921
Conversation
update nixpkgs to 24.05 fixup
409b54c
to
c47915a
Compare
This reverts commit d5dfdfb.
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.
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.
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.
crates/compiler/test_gen/src/helpers/wasm_linking_test_host.zig
Outdated
Show resolved
Hide resolved
// TODO: double check how much time is spent verifying here and below. | ||
// For real compilation, we may not want to pay the cost. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no
There was a problem hiding this comment.
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.
examples/cli was removed in #6921
No description provided.