elliottt edited PR #5222 from trevor/vreg-allocator
to main
:
Remove the dependency on
VCode
forVReg
allocation. This will simplify the changes in #5172, as that PR introduces the need to allocate temporary registers from the ABI context.This change also allows us to remove some fields from
VCode
:reftyped_vregs_set
andhave_ref_values
. Additionally, thevcode_types
field appears to be only used for its length after theVCode
is built, so we could probably replace that field with au32
as well.<!--
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.
-->
elliottt requested fitzgen for a review on PR #5222.
fitzgen submitted PR review.
fitzgen submitted PR review.
fitzgen created PR review comment:
FWIW, this could be pushed into
alloc<I>
, which is the only place where this is used, AFAICT. Would just make the type annotations a little simpler for other structs containing this type and remove the need for this phantom data. Not a big deal though.
elliottt submitted PR review.
elliottt created PR review comment:
I like that much more, thanks!
elliottt submitted PR review.
elliottt created PR review comment:
I gave this a try, and unfortunately it complicates the call sites of
alloc
, asI
can't be inferred from any of the arguments :(
elliottt merged PR #5222.
cfallin submitted PR review.
cfallin submitted PR review.
cfallin created PR review comment:
This is pre-existing code I know, but as long as I'm reading over it, I think we probably should use
types::INVALID
to extend theVec
rather than a default ofI8
.
elliottt submitted PR review.
elliottt created PR review comment:
That's a good point, I'll make a follow-up PR :)
elliottt submitted PR review.
elliottt created PR review comment:
#5227
Last updated: Dec 23 2024 at 12:05 UTC