-
Notifications
You must be signed in to change notification settings - Fork 1.7k
pulley: Implement interpreter-to-host calls #9665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -541,36 +541,29 @@ where | |
| insts | ||
| } | ||
|
|
||
| fn gen_call(dest: &CallDest, tmp: Writable<Reg>, info: CallInfo<()>) -> SmallVec<[Self::I; 2]> { | ||
| if info.callee_conv == isa::CallConv::Tail || info.callee_conv == isa::CallConv::Fast { | ||
| match &dest { | ||
| &CallDest::ExtName(ref name, RelocDistance::Near) => smallvec![Inst::Call { | ||
| info: Box::new(info.map(|()| name.clone())) | ||
| } | ||
| .into()], | ||
| &CallDest::ExtName(ref name, RelocDistance::Far) => smallvec![ | ||
| Inst::LoadExtName { | ||
| dst: WritableXReg::try_from(tmp).unwrap(), | ||
| name: Box::new(name.clone()), | ||
| offset: 0, | ||
| } | ||
| .into(), | ||
| Inst::IndirectCall { | ||
| info: Box::new(info.map(|()| XReg::new(tmp.to_reg()).unwrap())) | ||
| } | ||
| .into(), | ||
| ], | ||
| &CallDest::Reg(reg) => smallvec![Inst::IndirectCall { | ||
| info: Box::new(info.map(|()| XReg::new(*reg).unwrap())) | ||
| } | ||
| .into()], | ||
| fn gen_call( | ||
| dest: &CallDest, | ||
| _tmp: Writable<Reg>, | ||
| info: CallInfo<()>, | ||
| ) -> SmallVec<[Self::I; 2]> { | ||
| match dest { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also check the calling conventions at all here? That was what I was (hackily) using to distinguish between pulley-to-pulley and pulley-to-host before. I like reloc-distance better but maybe we should be asserting that pulley-to-pulley always uses
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's reasonable yeah, I'll try to go back and add some assertions. |
||
| // "near" calls are pulley->pulley calls so they use a normal "call" | ||
| // opcode | ||
| CallDest::ExtName(name, RelocDistance::Near) => smallvec![Inst::Call { | ||
| info: Box::new(info.map(|()| name.clone())) | ||
| } | ||
| } else { | ||
| todo!( | ||
| "host calls? callee_conv = {:?}; caller_conv = {:?}", | ||
| info.callee_conv, | ||
| info.caller_conv, | ||
| ) | ||
| .into()], | ||
| // "far" calls are pulley->host calls so they use a different opcode | ||
| // which is lowered with a special relocation in the backend. | ||
| CallDest::ExtName(name, RelocDistance::Far) => smallvec![Inst::IndirectCallHost { | ||
| info: Box::new(info.map(|()| name.clone())) | ||
| } | ||
| .into()], | ||
| // Indirect calls are all assumed to be pulley->pulley calls | ||
| CallDest::Reg(reg) => smallvec![Inst::IndirectCall { | ||
| info: Box::new(info.map(|()| XReg::new(*reg).unwrap())) | ||
| } | ||
| .into()], | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| test compile precise-output | ||
| target pulley64 | ||
|
|
||
| function %call_indirect_host() { | ||
| fn0 = u10:0() system_v | ||
| block0: | ||
| call fn0() | ||
| return | ||
| } | ||
|
|
||
| ; VCode: | ||
| ; x30 = xconst8 -16 | ||
| ; x27 = xadd32 x27, x30 | ||
| ; store64 sp+8, x28 // flags = notrap aligned | ||
| ; store64 sp+0, x29 // flags = notrap aligned | ||
| ; x29 = xmov x27 | ||
| ; block0: | ||
| ; indirect_call_host CallInfo { dest: User(userextname0), uses: [], defs: [], clobbers: PRegSet { bits: [65535, 65279, 4294967295, 0] }, callee_conv: SystemV, caller_conv: Fast, callee_pop_size: 0 } | ||
| ; x28 = load64_u sp+8 // flags = notrap aligned | ||
| ; x29 = load64_u sp+0 // flags = notrap aligned | ||
| ; x30 = xconst8 16 | ||
| ; x27 = xadd32 x27, x30 | ||
| ; ret | ||
| ; | ||
| ; Disassembled: | ||
| ; xconst8 spilltmp0, -16 | ||
| ; xadd32 sp, sp, spilltmp0 | ||
| ; store64_offset8 sp, 8, lr | ||
| ; store64 sp, fp | ||
| ; xmov fp, sp | ||
| ; call_indirect_host 0 | ||
| ; load64_offset8 lr, sp, 8 | ||
| ; load64 fp, sp | ||
| ; xconst8 spilltmp0, 16 | ||
| ; xadd32 sp, sp, spilltmp0 | ||
| ; ret | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the signature actually need to be resolved at reloc time? It can't be done at compile time and embedded in the instruction itself?
The address of any host function obviously needs to be reloc time (this is a bit of an aside because my understanding is that we aren't actually embedding any host function addresses in the pulley bytecode) however the signature doesn't seem like it should need to be resolved at reloc time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what you're thinking is already done actually, but the phrasing here is ambiguous. The "reloc time" technically happens twice -- once when linking things into artifacts and again when loading the artifacts. Putting the signature into the instruction happens in the first of these, during linking time. The relocation here is needed because the
UserExternalNameisn't available during compilation, only after the compile has finished, so that level of relocation processing is required to stuff it in.Otherwise though there's no runtime relocation when we load the bytecode itself, it's all frozen and loaded as-is from disk or the compile artifact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhhh it is the function's id/code that is being reloc'd at link time here? That makes sense to me. When I read "signature" I was thinking "parameter and result types" and perhaps "calling convention", which happens to align with
cranelift_codegen::ir::Signature.Can we replace "signature" with "code" or "id" in these bits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point yeah, this is also something that changed halfway through the design and I didn't get around to updating all the docs