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

x64: Missed optimization with fnmsub instruction #8953

Open
UnlimitedHummus opened this issue Jul 13, 2024 · 0 comments
Open

x64: Missed optimization with fnmsub instruction #8953

UnlimitedHummus opened this issue Jul 13, 2024 · 0 comments

Comments

@UnlimitedHummus
Copy link
Contributor

The PR #8888 added support for fmsub and fnmsub instructions to x64. This allows sinking loads and negations, which needed
to be executed when only the fmadd and fnmadd instructions where available. This improves code generation in most cases.
However, in cases where a value is both a sinkable load and negated, the order of those matters in the current implementation.
If the value is negated first then stored on the stack and loaded after, an fnmsub instruction could be generated, which would remove the need for explicit negation.

Feature

The fnmsub instruction should also be generated in cases where one of the multiplicands is negated first and then loaded.

Benefit

The generated code would be improved in this special case. By using the fnmsub instruction the multiplicand doesn't need to be negated first.

Implementation

There are test cases added in #8888 for adding both possible orders of negation and loading. Likely this can be achieved by adding additional rules to the lowering code. There would have to be rules for the special case, where negation happens first.

UnlimitedHummus added a commit to UnlimitedHummus/wasmtime that referenced this issue Jul 13, 2024
github-actions bot pushed a commit that referenced this issue Jul 13, 2024
* add fmsub and fnmsub instructions

* write tests for fmsub and fnmsub

* add a single runtest

* add more tests

* fix fmnsub_f32 test

* add reference to issue #8953
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

No branches or pull requests

1 participant