Closed dalaing closed 4 years ago
Cool! This looks good.
For administrative instructions I would go with whatever is simplest and cleanest. There is no need to keep them backwards-compatible, as they are merely an implementation detail of the spec formalism. I'm a bit surprised that you'd need several instructions, though. Wouldn't a single return_invoke
be sufficient?
A couple of comments regarding the tail call annotation on function types: The main type distinction currently being discussed is not whether a function can be tail-called, but whether a function can itself perform a tail call. (The motivation is that this might save passing an extra arity parameter to all functions on some engines.) Furthermore, if you want to extend function types with an annotation, then a bit more work is necessary: since function types are also used to type instructions, you would need to separate the two, because the annotation does not make sense for instructions. Checking the can-tail-call attribute also requires some work; I suspect you would need to add this information to contexts somehow. (Alternatively, one could use the annotated type for instructions and use this to infer tail-calling, instead of making it a constraint in the context a priori -- hmm...) Either way, I would suggest leaving out this annotation for now. We can easily add it once we have settled on what exactly implementers think is necessary.
One other remark: the tail-call repo wasn't up-to-date wrt the upstream spec repo. For your convenience, I just sync'ed it. Stuff might have moved around.
I've updated the repo and split out the work - the addition of the tail call instructions now appears here.
The extra instructions were the easy path to get things going. At the moment, the Return instruction uses the Br instruction to use the Break instruction to get things off the stack, and I've done something similar. I'm currently poking around with something to streamline what I've done.
The function type annotation I added was optional, defaulting to the non-tail-call case, in order to be compatible with the existing typing of instructions etc... It sounds like there are a few things that I haven't thought about on this front, but I'm happy to tackle that later on.
Oh, I see what you mean. In fact, I removed the way Return is reduced to Br from the formal semantics a while ago -- because it created a typing anomaly, and because it wouldn't generalise to tail calls ;). But I forgot to adapt the interpreter. I just did so, see WebAssembly/spec#658. I down-streamed that change to the tail-call repo.
The way I would go about adding tail calls now is by generalising the definition of Returning
to
Returning of code
such that e.g. ReturnCall x :: vs
reduces to
Returning (vs, [Call x])
or something like that. Though maybe it's easier to add a separate constructor ReturnCalling.
We have part of an implementation of this proposal over here. It adds two new call instructions and an optional annotation to function types to describe whether they are tail recursive or not. I believe the change is backwards compatible for existing code.
It needs more tests and documentation for the execution part, but other than that it seems to be coming together. I also need to remove some commented out code that shows me the stack depth as the interpreter runs.
I've made the execution part work with the addition of a couple of new administrative instructions. I'm about to try to tidy those up and document them (and then figure out what I need to do able host functions).
Before I do that, I thought I should ask in here: is there a preference between adding new administrative instructions and modifying the existing ones to carry more information?
I figure adding new instructions means that the proposal is self contained, whereas modifying existing administrative instructions risk having an effect elsewhere. I don't know what people's thoughts are with regard to whether backwards compatibility is more desirable / less desirable than keeping the number of administrative instructions low.
I haven't actually thought about how to modify the existing administrative instructions to do tail calls, but I can cross that bridge once I get to it.