elliottt opened PR #4542 from trevor/x64-lower-call
to main
:
- Migrate call and call_indirect to ISLE
- Update tests
<!--
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 updated PR #4542 from trevor/x64-lower-call
to main
.
elliottt updated PR #4542 from trevor/x64-lower-call
to main
.
elliottt edited PR #4542 from trevor/x64-lower-call
to main
:
Migrate the lowerings for
call
andcall_indirect
to ISLE. This is a less thorough embedding of those two lowerings in ISLE than the s390x backend, but we can re-evaluate that decision later.
<!--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 has marked PR #4542 as ready for review.
elliottt requested cfallin for a review on PR #4542.
cfallin submitted PR review.
cfallin submitted PR review.
cfallin created PR review comment:
It doesn't look like this is actually invoked from ISLE -- I guess it's here so it can be in the
Context
trait and so the impl can sit next to its user inisle.rs
?I think I'd prefer to move it to a separate
impl IsleContext
block (ie not the trait impl) and avoid listing it here, just to avoid confusion ("why is this an extractor, where is it used?") if for no other reason...
cfallin created PR review comment:
Let's have a
panic
here instead: in theory we could have a larger number if we have a narrower machine (I128
on a 32-bit architecture) or larger type. Otherwise we just fail isel in a hard-to-debug way...
elliottt created PR review comment:
When my previous attempt had more of the lowering implemented in ISLE, I was using it there. I agree that it's odd to see it here, only to be used from the rust side -- we can always add it back to the prelude if it turns out it's necessary.
elliottt submitted PR review.
elliottt updated PR #4542 from trevor/x64-lower-call
to main
.
elliottt updated PR #4542 from trevor/x64-lower-call
to main
.
elliottt merged PR #4542.
Last updated: Dec 23 2024 at 12:05 UTC