-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
add component-model-async/fused.wast test #10106
base: main
Are you sure you want to change the base?
Conversation
ecd9754
to
7e41099
Compare
This is another piece of bytecodealliance#9582 which I'm splitting out to make review easier. This test exercises fused adapter generation for various flavors of intercomponent async->async, async->sync, and sync->async calls. The remaining changes fill in some TODOs to make the test pass. Signed-off-by: Joel Dice <[email protected]>
7e41099
to
098bd1c
Compare
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 this is as far as I'm gonna get today. I haven't gotten to most of trampoline.rs
yet which I realize is most of the guts of this PR, but I'll do that on Monday
let params = self.types[self.signature] | ||
.unwrap_func() | ||
.params() | ||
.iter() | ||
.map(|&v| { | ||
Some(match v { | ||
WasmValType::I32 => FlatType::I32, | ||
WasmValType::I64 => FlatType::I64, | ||
WasmValType::F32 => FlatType::F32, | ||
WasmValType::F64 => FlatType::F64, | ||
_ => return None, | ||
}) | ||
}) | ||
.collect::<Option<_>>(); | ||
|
||
let ty = self.builder.ins().iconst( | ||
ir::types::I32, | ||
i64::from( | ||
params | ||
.and_then(|params| { | ||
self.types | ||
.get_task_return_type(&TypeTaskReturn { params }) | ||
.map(|v| v.as_u32()) | ||
}) | ||
.unwrap_or(u32::MAX), | ||
), | ||
); |
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.
Two things on this:
- For
let params = ...
above, it's unclear to me what this is doing. For example I'm not sure why this would switch toNone
for unknown types as opposed to return an error or panic or something. Additionally this looks like something that's better done in perhaps atranslate.rs
orinline.rs
step? - For
let ty = ...
this looks like it's doing a hash map lookup of a calculated type. Would it be possible to calculate that when the type is originally created early on intranslate.rs
and/orinline.rs
and then thread that through to here? For example I'd expect that this function here would havety: Option<SomeIndex>
as an argument and then that's what's encoded into au32
here.
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.
Agreed about the None
default; I don't remember why I did that. I'll switch it to a panic.
And yes, I can move this code to an earlier stage, e.g. translate.rs
or inline.rs
.
let call = self.compiler.call_indirect_host( | ||
&mut self.builder, | ||
index, | ||
host_sig, | ||
host_fn, | ||
&callee_args, | ||
); |
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.
Like above, the call_libcall
helper might be able to help clean this up a little further
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.
Per my response above: I've already done this in a follow-up PR.
This is another piece of #9582 which I'm splitting out to make review easier.
This test exercises fused adapter generation for various flavors of intercomponent async->async, async->sync, and sync->async calls.
The remaining changes fill in some TODOs to make the test pass.