fitzgen requested elliottt for a review on PR #5679.
fitzgen opened PR #5679 from tail-call-conv
to main
:
This is an in-progress implementation of https://github.com/bytecodealliance/rfcs/pull/29. The very non-controversial bits that just introduce a calling convention and tail call instructions, nothing that actually implements the calling conventions or supports lowering. As such, I think it doesn't really need to block on the RFC merging.
We also went down a little bit of a rabbit hole with DFG helper method refactoring. Sorry about the extra bits in the diff!
<!--
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 created PR review comment:
What do you think about inlining this in the verifier? All the fields used are public on the dfg, and there's currently only one call site.
elliottt submitted PR review.
elliottt submitted PR review.
jameysharp submitted PR review.
jameysharp edited PR review comment.
jameysharp created PR review comment:
Nick and I waffled over that. I kinda felt that it was nice to have it next to
DataFlowGraph::inst_result_types
since they have nearly identical structures. I don't feel super strongly about it though. I suspect there's some other way of factoring all this that would be even better but I haven't been able to figure out what that would be yet...
bjorn3 submitted PR review.
bjorn3 created PR review comment:
Can you add this on the other architectures too?
fitzgen submitted PR review.
fitzgen created PR review comment:
The other ISAs don't do exhaustive matches on the calling conventions, so there isn't anywhere to update.
fitzgen updated PR #5679 from tail-call-conv
to main
.
fitzgen updated PR #5679 from tail-call-conv
to main
.
fitzgen has enabled auto merge for PR #5679.
fitzgen merged PR #5679.
Last updated: Nov 22 2024 at 16:03 UTC