Skip to content

Change the result type of the unreachable operation to be the same as nop. #654

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

Closed
ghost opened this issue Apr 13, 2016 · 5 comments
Closed

Comments

@ghost
Copy link

ghost commented Apr 13, 2016

Allowing the result of the unreachable operation to be consumed introduces a type for this in a bottom up types system. Perhaps it could be modified to be invalid to use this operation in a context that expects a value. Perhaps this is equivalent to changing it to be effectively (new_unreachable) == (block (current_unreachable) (nop))? This might remove one type from the bottom up type system, remove the ANY type that some implementations are currently using?

It might also match it's use in a virtual instruction set in which it writes to no registers.

What do people think?

@rossberg
Copy link
Member

All unconditional control flow transfer operators (br, return, br_table, unreachable) have arbitrary type. That is standard behaviour in typed expression languages, and there is nothing special about unreachable in this regard. This semantics is necessary to allow these operators to appear on a branch where a sibling branch actually yields a value, such as

(if (<assertion>) (<result>) (unreachable))

@ghost
Copy link
Author

ghost commented Apr 14, 2016

@rossberg-chromium Thank you I forgot about br, return, and br_table. The proposal should be extended to these too. For example (new_br $l (exp)) == (block (current_br $l (exp)) (nop)) etc.

How necessary is it to allow these operators in places that expect a value? The only case that comes to mind might be the last expression in a block in the fall-through position, but I would need to think this through?

Even the proposed new_unreachable could be used with if so long as the consumer of if does not expect a value. For example, the following would be valid because the result of if does not expect a value.

(block (if (<assertion>) (<result>) (new_unreachable)) (...))

Just as the following is currently valid:

(block (if (<assertion>) (<result>) (block (current_unreachable) (nop))) (...))

If the consumer of if expected a value then the new_unreachable would not be valid here. In this case there would be some extra burden on the consumer to emit a value, for example:

(i32.eqz (if (<assertion>) (then (<result>)) (else (new_unreachable) (i32.const 0))))

Also perhaps it is reasonable to expect the above to be written as:

(block
  (if (i32.eqz (<assertion>)) (new_unreachable))
  (i32.eqz <result>))

Given the drive to keep the runtime as simple as possible, and the foot-gun that the unreachable type seems to be, this is the change I am proposing. Wouldn't it be nice to avoid the unreachable type? Might this be worth the small extra burden on consumers to emit the above work-around patterns to keep the type system happy?

Some counter use cases or examples would help if I am still missing the point?

I'll try making the change and test some code and see what breaks and what work-arounds would look like.

@rossberg
Copy link
Member

As I said, this is both useful and standard behaviour in expression languages: branches where one case produces a result value and the other aborts in some manner are not that rare, and there is no reason not to allow them. Why put artificial extra burden on producers?

Moreover, if we typed (return <expr>) with the nullary type, then you couldn't even use it at the end of a function anymore. That is, even

(func (result i32) (return (i32.const 0)))

would no longer type-check under your extended proposal.

@ghost
Copy link
Author

ghost commented Apr 14, 2016

@rossberg-chromium The example of a trailing return does not seem very compelling because it can be trivially re-written to (func (result i32) (i32.const 0)) and this is even a more efficient encoding. There might still be some compelling cases in this last-expression block position, and I'll keep exploring.

A. If the wasm principle is to strip the language to the core low level requirements while moving work and complexity to the producer then this change seems consistent with that principle. It seems strictly unnecessary to allow unreachable in a context accepting a value. The worst case workaround is (block (unreachable) (i32.const 0)) etc.

B. If implementers are prepared to trade off some complexity for more flexible expression support then the unreachable type is still unnecessary, rather we can extend the expression support to default unsupplied values, for example (i32.eqz (nop)) == (i32.eqz (i32.const 0)) which also gives us a short encoding for zero and would significantly simplify the multiple-value support too. Then the proposed definition of (new_unreachable) = (block (current_unreachable) (nop)) would be accepted anywhere that a value was consumed too.

C. The feedback that I see on the multiple-value support is that it will basically give up on the 'expression' pattern, have call save the results immediately in registers, that the page or two of extra runtime support code is too much of a burden. I see the end of expressions in wasm, except perhaps as an efficient encoding for some code sequences but even this might not be warranted if a register instruction set encoding were relatively competitive and/or if it is reasonable for the JS developer code to emit the register instruction set (and potentially use expressions in an upstream compressed encoding). Being able to remove the unreachable type might have some minor input into such decisions. I am exploring adding 'register' operators to wasm, it seems to be going well, they might be all that is needed for a MVP! Why do we need all the complexity of expressions for the MVP anyway?

@ghost
Copy link
Author

ghost commented Apr 16, 2016

Re-focusing on wasm without expressions where this is not relevant.

@ghost ghost closed this as completed Apr 16, 2016
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant