Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

More i8 legalizations #1253

Merged
merged 4 commits into from
Dec 4, 2019
Merged

Conversation

bjorn3
Copy link
Contributor

@bjorn3 bjorn3 commented Nov 23, 2019

  • A short description of what this does, why it is needed; if the
    description becomes long, the matter should probably be discussed in an issue
    first.
    This adds several legalizations for small integers.
  • This PR contains test cases, if meaningful.
  • A reviewer from the core maintainer team has been assigned for this PR.
    If you don't know who could review this, please indicate so and/or ping
    bnjbvr. The list of suggested reviewers on the right can help you.

Fixes #433.
cc #466 and #1117

@abrown
Copy link
Collaborator

abrown commented Dec 2, 2019

I think this PR makes sense to me but I'm not confident about how the legalization groups work (e.g. widen): @bnjbvr, any issues with this or should I approve and merge?

@abrown
Copy link
Collaborator

abrown commented Dec 2, 2019

(note to self: if we do merge we should probably "squash and merge")

@bnjbvr
Copy link
Member

bnjbvr commented Dec 2, 2019

I think @julian-seward1 proposed to give a look at this PR; I'll let the two of you coordinate for reviews.

@abrown abrown requested a review from julian-seward1 December 2, 2019 17:29
@julian-seward1
Copy link
Collaborator

I have to say, I don't understand the legalisation machinery enough to comment meaningfully here. I would however say that IMO we should try to avoid generating operations that read or write 16 bit registers on x86_{32,64} since that puts us at risk of partial register stalls.

@bjorn3
Copy link
Contributor Author

bjorn3 commented Dec 4, 2019

This makes some existing "macro" legalizations apply to small ints too. It also uses uextend to legalize brz.i8 to brnz.i32. There should be a faster way for this, but that can be implemented later.

@abrown abrown merged commit 1ad2aa0 into bytecodealliance:master Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing encodings: bint.i8, stack_store for i8
4 participants