This repository was archived by the owner on Jun 26, 2020. It is now read-only.
[codegen] add encodings for isub borrow variants#962
Merged
bnjbvr merged 3 commits intobytecodealliance:masterfrom Sep 5, 2019
Merged
[codegen] add encodings for isub borrow variants#962bnjbvr merged 3 commits intobytecodealliance:masterfrom
bnjbvr merged 3 commits intobytecodealliance:masterfrom
Conversation
Contributor
Author
|
Depends on #961. |
bnjbvr
reviewed
Sep 2, 2019
Member
bnjbvr
left a comment
There was a problem hiding this comment.
Same remark as for the add: the isub_ family should use iflags instead of b1. Can you tweak this here too, please?
9afad96 to
bd1663f
Compare
Contributor
Author
|
@bnjbvr this should work too, now. |
Member
|
Can you please rebase and autosquash? |
bnjbvr
reviewed
Sep 5, 2019
Member
bnjbvr
left a comment
There was a problem hiding this comment.
Looks correct, but I'd like to see the rebased version first.
filetests/isa/x86/legalize-i64.clif
Outdated
| ebb0(v1: i64, v2: i64): | ||
| v10 = iadd v1, v2 | ||
| ; check: v1 = iconcat $(v1_lsb=$V), $(v1_msb=$V) | ||
| ; check: v2 = iconcat $(v2_lsb=$V), $(v2_msb=$V) |
Member
There was a problem hiding this comment.
Actually, only the first line requires to be a check; next ones could be nextln, if the lines actually follow in the output. Can you try this, for all the tests in this file, and see if they still pass, please?
bd1663f to
fbb8655
Compare
Add encodings for isub borrow variants (isub_bout, isub_bin, isub_borrow) for x86_32, enabling the legalization for isub.i64 to work. Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1576675 Bug: https://github.com/CraneStation/cranelift/issues/765
Previously, the borrow variants of isub (isub_bin, isub_bout and isub_borrow) were being legalized for isa/riscv since RISC architectures lack a flags register. This forced us to return and accept booleans for these operations, which proved to be problematic and inconvenient, especially for x86. This commit removes support for said statements and all dependent statements for isa/riscv so that we can work on a better legalization strategy in the future.
…ants The type of the borrow operands for the borrow variants of the isub instruction (isub_bin, isub_bout, isub_borrow) was bool for compatibility reasons for isa/riscv. Since support for these instructions on RISC architectures has been temporarily suspended, we can safely change the type to iflags.
fbb8655 to
5383783
Compare
ryzokuken
added a commit
to ryzokuken/cranelift
that referenced
this pull request
Sep 9, 2019
…SC ISAs Reintroduce support for iadd carry variants and isub borrow variants for RISC ISAs which had been removed in bytecodealliance#961 and bytecodealliance#962 because of the lack of a proper flags register in RISC architectures.
ryzokuken
added a commit
to ryzokuken/cranelift
that referenced
this pull request
Sep 12, 2019
…SC ISAs Reintroduce support for iadd carry variants and isub borrow variants for RISC ISAs which had been removed in bytecodealliance#961 and bytecodealliance#962 because of the lack of a proper flags register in RISC architectures.
arkpar
pushed a commit
to paritytech/wasmtime
that referenced
this pull request
Mar 4, 2020
bytecodealliance#1005) Reintroduce support for iadd carry variants and isub borrow variants for RISC ISAs which had been removed in bytecodealliance/cranelift#961 and bytecodealliance/cranelift#962 because of the lack of a proper flags register in RISC architectures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add encodings for isub borrow variants (isub_bout, isub_bin, isub_borrow) for x86_32, enabling the legalization for isub.i64 to work.
Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1576675
Bug: https://github.com/CraneStation/cranelift/issues/765
/cc @bnjbvr @wingo @caitp