-
Notifications
You must be signed in to change notification settings - Fork 786
feat: add SKIP_OPTIMIZATIONS option to CMakeLists.txt #7569
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
base: main
Are you sure you want to change the base?
Conversation
I am unsure if it makes sense to land this here in upstream. If this were a common use case I'd feel differently, but I haven't heard of this situation before. And while the actual changes are not a large burden, they do add some complexity. |
A possible use case of this is to use Binaryen as a part of your own compiler targeting wasm. Wasm itself is being actively developed now and I think it may be used even for learning purposes (for example, as a part of compilers course). These changes make it more convenient to use code generation only (faster compilation, less unused code). In other words, I see the following flow, which separates compilation and optimisation.
@kripken, what do you think of the use case I mentioned? Does it seem irrelevant? Should I change something in the PR so that the changes look more useful for target user described above (maybe change name of the parameter, more comments, …)? |
What is the benefit of the separation? If you are are already doing 1, then you can tell binaryen to also optimize. That's just one line in the C API for example, and it is more efficient than doing optimizations separately. And if you are separating 1 and 2 but using wasm-opt in part 2, then you are still using all of binaryen, so this PR won't help? I can see a use case where you just don't want step 2, so you aren't optimizing at all. But this PR only helps by reducing the build size of binaryen by a few MB, which I'm not sure is a common issue? (I don't recall other people mentioning it.) |
The benefit is that you are able to compile many things into wasm modules (possibly in parallel), then link them together via something like |
If you are using all of wasm-opt at the end, why do you want a trimmed-down version of it as well? (Why not use the same wasm-opt for the early compilation and the late optimization?) |
In my opinion, this is better for logically distinguishing 3 different steps of the process and for (possibly) better parallelism. This allows using compiler-backend-only tool during (paralleled?) compilation without the need of loading more code into memory. And then you may use (or not use, if you don’t need it, e. g. for debugging/learning purpose) the optimizer during a separate process possibly on different machine, which only gets the entire compiled&linked file and does just the optimization. |
If the compile time is a problem for your use case, can you just use the release binaries instead? |
In short, it is not compile time only, the file size/amount of files/compexity also matters. Let me provide more general use case which is similar to mine.
In other words, I think changes like these may make Binaryen better as a tool for learning wasm/compilation into wasm and/or for faster MVPs for projects related to wasm. Does it seem irrelevant to what is Binaryen meant for? |
These are valid use cases, and Binaryen may be relevant there. What I am not sure of is the benefit of saving some binary size in one part of that story. And there are downsides to making two separate Binaryen builds just to save size in one of them, like a person might try using the smaller one to optimize and get confused why things don't work. I would change my mind if I saw benchmarks that show the speedup is very significant, and there are multiple users that are asking for this. For now, I think this doesn't make sense here in upstream. |
This is a naming issue, I think. If the build is named with something like
Ok, I see. Will try to provide some benchmarks later. And then let’s wait for someone else to start asking for this. At least, there is a way to do this in my fork. |
Added an option named
SKIP_OPTIMIZATIONS
to help resolve #7566