Skip to content

Fix uextend on x64 for non-i32-source cases.#3906

Merged
cfallin merged 2 commits intobytecodealliance:mainfrom
cfallin:uextend-sextend-fix
Mar 9, 2022
Merged

Fix uextend on x64 for non-i32-source cases.#3906
cfallin merged 2 commits intobytecodealliance:mainfrom
cfallin:uextend-sextend-fix

Conversation

@cfallin
Copy link
Member

@cfallin cfallin commented Mar 9, 2022

In #3849, I moved uextend over to ISLE in the x64 backend. Unfortunately, the lowering patterns had a bug in the i32-to-i64 special case (when we know the generating instruction zeroes the upper 32 bits): it wasn't actually special casing for an i32 source! This meant that e.g. zero extends of the results of i8 adds did not work properly.

This PR fixes the bug and updates the runtest for extends significantly to cover the narrow-value cases.

No security impact to Wasm as Wasm does not use narrow integer types.

Thanks @bjorn3 for reporting!

@cfallin cfallin requested a review from fitzgen March 9, 2022 18:15
Copy link
Contributor

@bjorn3 bjorn3 left a comment

Choose a reason for hiding this comment

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

I can confirm this fixes compiling snap with cg_clif.

@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:x64 Issues related to x64 codegen isle Related to the ISLE domain-specific language labels Mar 9, 2022
@github-actions
Copy link

github-actions bot commented Mar 9, 2022

Subscribe to Label Action

cc @cfallin, @fitzgen

Details This issue or pull request has been labeled: "cranelift", "cranelift:area:x64", "isle"

Thus the following users have been cc'd because of the following labels:

  • cfallin: isle
  • fitzgen: isle

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@cfallin cfallin merged commit 26ce9a3 into bytecodealliance:main Mar 9, 2022
@cfallin cfallin deleted the uextend-sextend-fix branch March 9, 2022 19:11
@bjorn3
Copy link
Contributor

bjorn3 commented Mar 9, 2022

Is a patch release planned?

@cfallin
Copy link
Member Author

cfallin commented Mar 9, 2022

@bjorn3 yes, I'll go ahead and push the button to start the patch release process.

cfallin added a commit to cfallin/wasmtime that referenced this pull request Mar 9, 2022
In bytecodealliance#3849, I moved uextend over to ISLE in the x64 backend. Unfortunately, the lowering patterns had a bug in the i32-to-i64 special case (when we know the generating instruction zeroes the upper 32 bits): it wasn't actually special casing for an i32 source! This meant that e.g. zero extends of the results of i8 adds did not work properly.

This PR fixes the bug and updates the runtest for extends significantly to cover the narrow-value cases.

No security impact to Wasm as Wasm does not use narrow integer types.

Thanks @bjorn3 for reporting!
cfallin added a commit that referenced this pull request Mar 9, 2022
* Fix uextend on x64 for non-i32-source cases. (#3906)

In #3849, I moved uextend over to ISLE in the x64 backend. Unfortunately, the lowering patterns had a bug in the i32-to-i64 special case (when we know the generating instruction zeroes the upper 32 bits): it wasn't actually special casing for an i32 source! This meant that e.g. zero extends of the results of i8 adds did not work properly.

This PR fixes the bug and updates the runtest for extends significantly to cover the narrow-value cases.

No security impact to Wasm as Wasm does not use narrow integer types.

Thanks @bjorn3 for reporting!

* Updated RELEASES for 0.35.1 patch release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator isle Related to the ISLE domain-specific language

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants