cfallin edited PR #1852 from reftypes
to main
:
This PR adds the inital support to allow reftypes to flow through the
program when targetting aarch64. It also adds a fix to the
ModuleTranslationState
needed to send R32/R64 types over from the
SpiderMonkey embedding.This PR does not include any support for safepoints in aarch64 or the
MachInst
infrastructure. That work will come as a second PR, following
some needed changes to regalloc.rs.This PR also makes a drive-by improvement to
Bint
, avoiding an
unneeded zero-extension op when the extended value comes directly from a
conditional-set (which produces a full-width 0 or 1).With this PR, and a companion patch in SpiderMonkey, we're able to make it
through all of the reftypes tests in the spec testsuite, though we aren't emitting
stackmaps yet so if the GC had been triggered we would have been in trouble.
Still, it's a start!<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] 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 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. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin edited PR #1852 from reftypes
to main
:
This PR adds support for reference types in the
MachInst
backend framework and on AArch64. It comes in two parts: the initial support for allowingR32
/R64
types to flow through the Wasm program and call boundaries, and the implementation of opcodes that support them (is_null
, etc.); and then stackmap support to allow GCs to occur while Wasm stackframes are active.This has been tested with SpiderMonkey and is known to work there with GCs occuring during calls from Wasm into JS. We plan to do further testing in regalloc.rs directly (with our checker) too.
This requires a regalloc.rs version bump (with wasmtime/regalloc.rs#79 included) and dep version update; the CI build here will be red until that change is made.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] 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 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. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin edited PR #1852 from reftypes
to main
:
This PR adds support for reference types in the
MachInst
backend framework and on AArch64. It comes in two parts: the initial support for allowingR32
/R64
types to flow through the Wasm program and call boundaries, and the implementation of opcodes that support them (is_null
, etc.); and then stackmap support to allow GCs to occur while Wasm stackframes are active.This has been tested with SpiderMonkey and is known to work there with GCs occuring during calls from Wasm into JS. We plan to do further testing in regalloc.rs directly (with our checker) too.
This requires a regalloc.rs version bump (with bytecodealliance/regalloc.rs#79 included) and dep version update; the CI build here will be red until that change is made.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] 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 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. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin updated PR #1852 from reftypes
to main
:
This PR adds support for reference types in the
MachInst
backend framework and on AArch64. It comes in two parts: the initial support for allowingR32
/R64
types to flow through the Wasm program and call boundaries, and the implementation of opcodes that support them (is_null
, etc.); and then stackmap support to allow GCs to occur while Wasm stackframes are active.This has been tested with SpiderMonkey and is known to work there with GCs occuring during calls from Wasm into JS. We plan to do further testing in regalloc.rs directly (with our checker) too.
This requires a regalloc.rs version bump (with bytecodealliance/regalloc.rs#79 included) and dep version update; the CI build here will be red until that change is made.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] 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 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. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin updated PR #1852 from reftypes
to main
:
This PR adds support for reference types in the
MachInst
backend framework and on AArch64. It comes in two parts: the initial support for allowingR32
/R64
types to flow through the Wasm program and call boundaries, and the implementation of opcodes that support them (is_null
, etc.); and then stackmap support to allow GCs to occur while Wasm stackframes are active.This has been tested with SpiderMonkey and is known to work there with GCs occuring during calls from Wasm into JS. We plan to do further testing in regalloc.rs directly (with our checker) too.
This requires a regalloc.rs version bump (with bytecodealliance/regalloc.rs#79 included) and dep version update; the CI build here will be red until that change is made.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] 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 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. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin updated PR #1852 from reftypes
to main
:
This PR adds support for reference types in the
MachInst
backend framework and on AArch64. It comes in two parts: the initial support for allowingR32
/R64
types to flow through the Wasm program and call boundaries, and the implementation of opcodes that support them (is_null
, etc.); and then stackmap support to allow GCs to occur while Wasm stackframes are active.This has been tested with SpiderMonkey and is known to work there with GCs occuring during calls from Wasm into JS. We plan to do further testing in regalloc.rs directly (with our checker) too.
This requires a regalloc.rs version bump (with bytecodealliance/regalloc.rs#79 included) and dep version update; the CI build here will be red until that change is made.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] 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 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. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin updated PR #1852 from reftypes
to main
:
This PR adds support for reference types in the
MachInst
backend framework and on AArch64. It comes in two parts: the initial support for allowingR32
/R64
types to flow through the Wasm program and call boundaries, and the implementation of opcodes that support them (is_null
, etc.); and then stackmap support to allow GCs to occur while Wasm stackframes are active.This has been tested with SpiderMonkey and is known to work there with GCs occuring during calls from Wasm into JS. We plan to do further testing in regalloc.rs directly (with our checker) too.
This requires a regalloc.rs version bump (with bytecodealliance/regalloc.rs#79 included) and dep version update; the CI build here will be red until that change is made.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] 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 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. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin updated PR #1852 from reftypes
to main
:
This PR adds support for reference types in the
MachInst
backend framework and on AArch64. It comes in two parts: the initial support for allowingR32
/R64
types to flow through the Wasm program and call boundaries, and the implementation of opcodes that support them (is_null
, etc.); and then stackmap support to allow GCs to occur while Wasm stackframes are active.This has been tested with SpiderMonkey and is known to work there with GCs occuring during calls from Wasm into JS. We plan to do further testing in regalloc.rs directly (with our checker) too.
This requires a regalloc.rs version bump (with bytecodealliance/regalloc.rs#79 included) and dep version update; the CI build here will be red until that change is made.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] 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 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. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin updated PR #1852 from reftypes
to main
:
This PR adds support for reference types in the
MachInst
backend framework and on AArch64. It comes in two parts: the initial support for allowingR32
/R64
types to flow through the Wasm program and call boundaries, and the implementation of opcodes that support them (is_null
, etc.); and then stackmap support to allow GCs to occur while Wasm stackframes are active.This has been tested with SpiderMonkey and is known to work there with GCs occuring during calls from Wasm into JS. We plan to do further testing in regalloc.rs directly (with our checker) too.
This requires a regalloc.rs version bump (with bytecodealliance/regalloc.rs#79 included) and dep version update; the CI build here will be red until that change is made.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] 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 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. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin updated PR #1852 from reftypes
to main
:
This PR adds support for reference types in the
MachInst
backend framework and on AArch64. It comes in two parts: the initial support for allowingR32
/R64
types to flow through the Wasm program and call boundaries, and the implementation of opcodes that support them (is_null
, etc.); and then stackmap support to allow GCs to occur while Wasm stackframes are active.This has been tested with SpiderMonkey and is known to work there with GCs occuring during calls from Wasm into JS. We plan to do further testing in regalloc.rs directly (with our checker) too.
This requires a regalloc.rs version bump (with bytecodealliance/regalloc.rs#79 included) and dep version update; the CI build here will be red until that change is made.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] 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 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. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
julian-seward1 submitted PR Review.
julian-seward1 submitted PR Review.
julian-seward1 created PR Review Comment:
nit: "is right here" is hard to parse. Does it mean "is correct here"? Or "at this point in the frame construction, nominal SP = actual SP" etc? Can you clarify?
julian-seward1 created PR Review Comment:
Errr,
*end*
is too vague. Please specify exactly. Do you mean: the code offset for the last (highest-addressed) byte of the instruction ..
julian-seward1 created PR Review Comment:
Can it happen that
Some(t)
is inconsistent with the second (here, ignored) component of the tuple? It would be good to add a comment explaining the meaning of this function. In particular why the first component has priority.
julian-seward1 created PR Review Comment:
nit:
..rc
feels a bit too terse here; can you call itref_type_regclass
?
julian-seward1 created PR Review Comment:
How is this used? In particular, will we have to call
is_safepoint
on every insn that is processed? That would seem to be an undesirable compiler performance hit, if so? (Tell me it ain't so!)
julian-seward1 created PR Review Comment:
nano-nit: since most of our users will be coming at this on 64-bit targets, maybe put that first in the disjunction? That way we can save perhaps 1/3 of a nanosecond per call :-)
julian-seward1 created PR Review Comment:
Here and below, it concerns me that we are accepting
R32
on a 64-bit target without comment. IIUC, if anything claiming to beR32
turns up on a 64-bit target, we have gone way off the rails -- perhaps even to the extent of having a security problem. In such a circumstance I would much prefer that we bring the system to a halt, even in release builds (== panic in every suchmatch
onty
s); and similarly for x64 stubs, if you have similarly modified them.
cfallin updated PR #1852 from reftypes
to main
:
This PR adds support for reference types in the
MachInst
backend framework and on AArch64. It comes in two parts: the initial support for allowingR32
/R64
types to flow through the Wasm program and call boundaries, and the implementation of opcodes that support them (is_null
, etc.); and then stackmap support to allow GCs to occur while Wasm stackframes are active.This has been tested with SpiderMonkey and is known to work there with GCs occuring during calls from Wasm into JS. We plan to do further testing in regalloc.rs directly (with our checker) too.
This requires a regalloc.rs version bump (with bytecodealliance/regalloc.rs#79 included) and dep version update; the CI build here will be red until that change is made.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] 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 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. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin submitted PR Review.
cfallin created PR Review Comment:
Done, excellent point.
cfallin submitted PR Review.
cfallin created PR Review Comment:
Sorry, this was left over from an earlier iteration that did indeed call
is_safepoint
on every instruction; but we no longer do that, and the function was dead, so I removed it.
cfallin submitted PR Review.
cfallin created PR Review Comment:
Updated comment, thanks!
cfallin submitted PR Review.
cfallin created PR Review Comment:
The type should always be consistent with the register class; the
Option<Type>
is only missing when the regalloc requests a spill of a realreg (for which it doesn't have a vreg, hence no vcode-level type) at a safepoint, and this will always be anI64
reg. Added comments to clarify.
cfallin merged PR #1852.
Last updated: Dec 23 2024 at 12:05 UTC