Skip to content

Conversation

@aheejin
Copy link
Member

@aheejin aheejin commented Feb 1, 2024

This adds --experimental-new-eh option to wasm-opt. The difference between this and --translate-to-new-eh is, --translate-to-new-eh just runs TranslateToNewEH pass, while --experimental-new-eh attaches TranslateToNewEH pass at the end of the whole optimization pipeline. So if no other passes or optimization options (-On) are specified, it is equivalent to --translate-to-new-eh. If other optimization passes are specified, it runs them and at the end run the translator to ensure the new EH instructions are emitted. The reason we are doing this this way is that the optimization pipeline as a whole does not support the new EH instruction yet, but we would like to provide an option to emit a reasonably OK code with the new EH instructions.

This also means when the optimization level > 3, it will also run the StackIR optimization after the translation.

Not sure how to test the output of this option, given that there is not much point in testing the default optimization passes, and it is also not clear how to print the stack IR if the stack ir generation and optimization runs as a part of the pipeline and not the explicit command line options.

This adds `--experimental-new-eh` option to `wasm-opt`. The difference
between this and `--translate-to-new-eh` is, `--translate-to-new-eh`
just runs `TranslateToNewEH` pass, while `--experimental-new-eh`
attaches `TranslateToNewEH` pass at the end of the whole optimization
pipeline. So if no other passes or optimization options (`-On`) are
specified, it is equivalent to `--translate-to-new-eh`. If other
optimization passes are specified, it runs them and at the end run the
translator to ensure the new EH instructions are emitted. The reason we
are doing this this way is that the optimization pipeline as a whole
does not support the new EH instruction yet, but we would like to
provide an option to emit a reasonably OK code with the new EH
instructions.

This also means when the optimization level > 3, it will also run
the StackIR optimization after the translation.

Not sure how to test the output of this option, given that there is not
much point in testing the default optimization passes, and it is also
not clear how to print the stack IR if the stack ir generation and
optimization runs as a part of the pipeline and not the explicit command
line options.
@aheejin aheejin requested review from kripken and tlively February 1, 2024 01:20
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

What is the long-term plan for this flag? Will it eventually mean that wasm-opt will run the translation pass at the beginning of the pipeline instead? Will this flag be eventually removed?

@aheejin
Copy link
Member Author

aheejin commented Feb 1, 2024

What is the long-term plan for this flag? Will it eventually mean that wasm-opt will run the translation pass at the beginning of the pipeline instead?

Yes, after the optimization pipeline is ready, the translator will run at the beginning of the pipeline to handle the old binaries in the wild.

Will this flag be eventually removed?

I think eventually it should.

addIfNoDWARFIssues("directize");

// Translate to use the new EH instructions if requested.
if (wasm->features.hasExceptionHandling() && options.experimentalNewEH) {
Copy link
Member

Choose a reason for hiding this comment

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

Doing this here means it will run twice if the user does wasm-opt -O3 -O3. Perhaps it would be better to insert this pass in src/tools/wasm-opt.cpp? Then we can add it right at the end.

Copy link
Member

Choose a reason for hiding this comment

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

A downside to my idea is that we'd need to also add logic there for adding the stackIR passes right after it, copying the 4 lines underneath us. But I think it might still be better.

Copy link
Member Author

@aheejin aheejin Feb 1, 2024

Choose a reason for hiding this comment

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

Then if a user gives wasm-opt --generate-stack-ir --optimize-stack-ir --experimental-new-eh (or -O3 --experimental-new-eh, which I guess also runs the stack IR generation and optimization), aren't we gonna generate and optimize the stack IR, throw it away, run the translator, and re-generate and re-optimize the stack IR?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. We also do that with -O3 -O3, as the end of each of them generates stack IR and optimizes it. A little wasteful, but not significant (making the pass pipeline smarter to avoid this overhead likely isn't worth it).

Copy link
Member Author

@aheejin aheejin Feb 1, 2024

Choose a reason for hiding this comment

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

Is -O3 -O3 common enough? I thought that would only happen if someone carelessly adds it twice, not remembering they have already done it, but I might be wrong.. whereas I thought -O3 --experimental-new-eh would be pretty common, because it's basically just -O3 with the intention of using the new EH, which now runs stack IR generation/optimization twice.

Copy link
Member Author

Choose a reason for hiding this comment

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

Chatted w/ @tlively and I learned people run -O3 not only twice but many times... Yeah, it'd be better to run it only once at the end of wasm-opt, even though it means running Stack IR twice.

Copy link
Member Author

Choose a reason for hiding this comment

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

Closing this in favor of #6270, which adds the option directly to wasm-opt.cpp.

aheejin added a commit to aheejin/binaryen that referenced this pull request Feb 2, 2024
This adds `--experimental-new-eh` option to `wasm-opt`. The difference
between this and `--translate-to-new-eh` is, `--translate-to-new-eh`
just runs `TranslateToNewEH` pass, while `--experimental-new-eh`
attaches `TranslateToNewEH` pass at the end of the whole optimization
pipeline. So if no other passes or optimization options (`-On`) are
specified, it is equivalent to `--translate-to-new-eh`. If other
optimization passes are specified, it runs them and at the end run the
translator to ensure the new EH instructions are emitted. The reason we
are doing this this way is that the optimization pipeline as a whole
does not support the new EH instruction yet, but we would like to
provide an option to emit a reasonably OK code with the new EH
instructions.

This also means when the optimization level > 3, it will also run
the StackIR + local2stack optimization after the translation.

Not sure how to test the output of this option, given that there is not
much point in testing the default optimization passes, and it is also
not clear how to print the stack IR if the stack ir generation and
optimization runs as a part of the pipeline and not the explicit command
line options.

This is created in favor of WebAssembly#6267, which added the option to
`optimization-options.h`. This had a problem of running the translator
multiple times when `-On` was given multiple times in the command line,
which I learned was rather a common usage. This adds the option directly
to `wasm-opt.cpp`, which avoids the problem. With this, it is still
possible to create and optimize Stack IR unnecessarily, but that feels a
better alternative.
@aheejin aheejin closed this Feb 2, 2024
@aheejin aheejin deleted the new_eh_option branch February 2, 2024 00:05
aheejin added a commit that referenced this pull request Feb 6, 2024
This adds `--experimental-new-eh` option to `wasm-opt`. The difference
between this and `--translate-to-new-eh` is, `--translate-to-new-eh`
just runs `TranslateToNewEH` pass, while `--experimental-new-eh`
attaches `TranslateToNewEH` pass at the end of the whole optimization
pipeline. So if no other passes or optimization options (`-On`) are
specified, it is equivalent to `--translate-to-new-eh`. If other
optimization passes are specified, it runs them and at the end run the
translator to ensure the new EH instructions are emitted. The reason we
are doing this this way is that the optimization pipeline as a whole
does not support the new EH instruction yet, but we would like to
provide an option to emit a reasonably OK code with the new EH
instructions.

This also means when the optimization level > 3, it will also run
the StackIR + local2stack optimization after the translation.

Not sure how to test the output of this option, given that there is not
much point in testing the default optimization passes, and it is also
not clear how to print the stack IR if the stack ir generation and
optimization runs as a part of the pipeline and not the explicit command
line options.

This is created in favor of #6267, which added the option to
`optimization-options.h`. It had a problem of running the translator
multiple times when `-On` was given multiple times in the command line,
which I learned was rather a common usage. This adds the option directly
to `wasm-opt.cpp`, which avoids the problem. With this, it is still
possible to create and optimize Stack IR unnecessarily, but that feels a
better alternative.
radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
This adds `--experimental-new-eh` option to `wasm-opt`. The difference
between this and `--translate-to-new-eh` is, `--translate-to-new-eh`
just runs `TranslateToNewEH` pass, while `--experimental-new-eh`
attaches `TranslateToNewEH` pass at the end of the whole optimization
pipeline. So if no other passes or optimization options (`-On`) are
specified, it is equivalent to `--translate-to-new-eh`. If other
optimization passes are specified, it runs them and at the end run the
translator to ensure the new EH instructions are emitted. The reason we
are doing this this way is that the optimization pipeline as a whole
does not support the new EH instruction yet, but we would like to
provide an option to emit a reasonably OK code with the new EH
instructions.

This also means when the optimization level > 3, it will also run
the StackIR + local2stack optimization after the translation.

Not sure how to test the output of this option, given that there is not
much point in testing the default optimization passes, and it is also
not clear how to print the stack IR if the stack ir generation and
optimization runs as a part of the pipeline and not the explicit command
line options.

This is created in favor of WebAssembly#6267, which added the option to
`optimization-options.h`. It had a problem of running the translator
multiple times when `-On` was given multiple times in the command line,
which I learned was rather a common usage. This adds the option directly
to `wasm-opt.cpp`, which avoids the problem. With this, it is still
possible to create and optimize Stack IR unnecessarily, but that feels a
better alternative.
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.

3 participants