uweigand opened PR #4598 from s390x-i128
to main
:
This adds full i128 support to the s390x target, including new filetests
and enabling the existing i128 runtest on s390x.The ABI requires that i128 is passed and returned via implicit pointer,
but the front end still generates direct i128 types in call. This means
we have to implement ABI support to implicitly convert i128 types to
pointers when passing arguments.To do so, we add a new variant ABIArg::ImplicitArg. This acts like
StructArg, except that the value type is the actual target type,
not a pointer type. The required conversions have to be inserted
in the prologue and at function call sites.Note that when dereferencing the implicit pointer in the prologue,
we may require a temp register: the pointer may be passed on the
stack so it needs to be loaded first, but the value register may
be in the wrong class for pointer values. In this case, we use
the "stack limit" register, which should be available at this
point in the prologue.For return values, we use a mechanism similar to the one used for
supporting multiple return values in the Wasmtime ABI. The only
difference is that the hidden pointer to the return buffer must
be the first, not last, argument in this case.(FYI @cfallin - This implements the second half of issue #4565.)
<!--
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 created PR review comment:
As an alternative I suppose we could provide an "allocate temp vreg" callback to
gen_copy_arg_to_regs
; this codegen happens before regalloc so it is still an option to create vregs here. Perhaps a comment here to record the altnernate possibility? (We may in the future want to remove the need for more dedicated non-allocatable registers, at which point it may become more relevant.)
cfallin created PR review comment:
Can we call this an
ImplicitPtrArg
or similar? JustImplicitArg
seems slightly ambiguous to me; like the arg itself is implicitly generated at callsites or something like that.
cfallin created PR review comment:
Comment somewhere in here that we write
tmp
only after the last reads ofrn
andrm
, as required by the semantics of ordinary (early) uses and ordinary (late) defs? And perhaps a note that if this changes,tmp
has to become an early def. (I mostly want to future-proof against a subtle mistake later.)
cfallin created PR review comment:
Add a note here that s390x is not yet supported because (reason)?
cfallin submitted PR review.
cfallin submitted PR review.
uweigand updated PR #4598 from s390x-i128
to main
.
uweigand submitted PR review.
uweigand created PR review comment:
Done
uweigand submitted PR review.
uweigand created PR review comment:
Done
uweigand created PR review comment:
Ah yes, that's better. However, I wasn't able to get a callback past the borrow checker - maybe I'm just not experienced enough with Rust, but it does look a real problem:
for insn in self.vcode.abi().gen_copy_arg_to_regs(i, regs).into_iter() {
This already holds a borrow on
self
during thegen_copy_arg_to_regs
call. Passing a closure would require another (mutable) borrow onself
, which isn't allowed.Instead, I noticed that there is already a mechanism to pass a temp reg to
ABICallee
via thetemp_needed
/init
mechanism, so I simply extended that to support multiple temps.
uweigand submitted PR review.
uweigand submitted PR review.
uweigand created PR review comment:
Done
cfallin submitted PR review.
cfallin has enabled auto merge for PR #4598.
cfallin merged PR #4598.
Last updated: Dec 23 2024 at 12:05 UTC