Skip to content

handling dead code #90

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
binji opened this issue Sep 29, 2015 · 6 comments
Closed

handling dead code #90

binji opened this issue Sep 29, 2015 · 6 comments

Comments

@binji
Copy link
Member

binji commented Sep 29, 2015

@AndrewScheidecker and I were chatting yesterday about the expression value of the return statement, specifically should something like this be allowed:

...
(f32.neg (return (f32.const 1.0)))

This looks like an error to me, and sexpr-wasm-prototype and v8-native-prototype treat is an error. But it is currently allowed in ml-proto. @sunfishcode also suggested that it might be useful to allow dead code for non-optimizing wasm.

Thoughts?

@lukewagner
Copy link
Member

Do sexpr-wasm-prototype or v8-native-prototype have the stmt=expr unification? I thought they didn't, which is why that line would be an error. However, even without stmt=expr, you run into the same question, just at statement level ((block (return) (call $foo))). Since I don't think dead code is a big problem for engines (from Odin experience) and since it doesn't add value to code generators (and, as @sunfishcode pointed out, it could be a burden) and since it's more work to specify, I'd side with not doing anything to disallow dead code.

@binji
Copy link
Member Author

binji commented Sep 29, 2015

I guess dead code in general is probably not worth preventing, but the particular case using the value of a return still seems strange to me. For example, this fails with a type mismatch error:

(func (result i32)
  (f32.neg (return (i32.const 1))))

clearly the type of a return statement is assumed to be the type of its child expression, but that doesn't really make any sense to me.

@rossberg
Copy link
Member

I guess dead code in general is probably not worth preventing, but the particular case using the value of a return still seems strange to me. For example, this fails with a type mismatch error:

(func (result i32)
(f32.neg (return (i32.const 1))))

clearly the type of a return statement is assumed to be the type of its child expression, but that doesn't really make any sense to me.

No, the type of a return statement is polymorphic, i.e., whatever the
surrounding expression expects. It is unrelated to its operand. The
type mismatch in your example is between the outer expression and the
function return type. This variant type-checks just fine:

(func (result i32)
  (i32.round_s/f32 (f32.neg (return (i32.const 1))))
)

As an expression, a return must be allowed to have any type. This is
to support patterns like (if (c) (return (a)) (b)), where the type
of both branches must match. The same applies to all other non-local
control-transfer constructs. That is standard behaviour in expression
languages.

I'm not sure what the issue is here. There are a million ways to write
dead code, why do we care about this one in particular?

@titzer
Copy link
Contributor

titzer commented Sep 30, 2015

On Wed, Sep 30, 2015 at 2:07 PM, rossberg-chromium <[email protected]

wrote:

I guess dead code in general is probably not worth preventing, but the
particular case using the value of a return still seems strange to me. For
example, this fails with a type mismatch error:

(func (result i32)
(f32.neg (return (i32.const 1))))

clearly the type of a return statement is assumed to be the type of its
child expression, but that doesn't really make any sense to me.

No, the type of a return statement is polymorphic, i.e., whatever the
surrounding expression expects. It is unrelated to its operand. The
type mismatch in your example is between the outer expression and the
function return type. This variant type-checks just fine:

(func (result i32)
(i32.round_s/f32 (f32.neg (return (i32.const 1))))
)

So the expression `if(c) (break[M] ) (break[N] )" typechecks in
call contexts where label M expects i32 and N expects f64?

As an expression, a return must be allowed to have any type. This is
to support patterns like (if (c) (return (a)) (b)), where the type
of both branches must match. The same applies to all other non-local
control-transfer constructs. That is standard behaviour in expression
languages.

I'm not sure what the issue is here. There are a million ways to write
dead code, why do we care about this one in particular?


Reply to this email directly or view it on GitHub
#90 (comment).

@rossberg
Copy link
Member

So the expression `if(c) (break[M] ) (break[N] )" typechecks in
call contexts where label M expects i32 and N expects f64?

Yes.

@binji
Copy link
Member Author

binji commented Sep 30, 2015

OK, makes sense.

@binji binji closed this as completed Sep 30, 2015
eqrion pushed a commit to eqrion/wasm-spec that referenced this issue Jul 18, 2019
alexcrichton pushed a commit to alexcrichton/spec that referenced this issue Nov 18, 2019
Issue in Shuffle
s/result[i] = b[s[i] - S.lanes]/result[i] = b[s[i] - S.lanes

Issue in Bit Shifts
s/def S.shl(a, x):/def S.shl(a, y):
Connicpu pushed a commit to Connicpu/wasm-spec that referenced this issue May 11, 2020
* [spec] Normative: Store is per agent cluster

With the WebAssembly threads proposal, a single Memory can be shared between
multiple agents within an agent cluster. This patch associates a WebAssembly
store with an agent cluster, rather than an agent, and updates the WebAssembly
JS API to define the store for an agent as the store for its surrounding
agent cluster.

* [spec] Normative: Define serialization for SAB Memory

The semantics here are
- forStorage serialization is disallowed
- Transfers are only permitted within an agent cluster
- Transfer of Memory backed by SharedArrayBuffers makes the buffer available in
  the source and the destination, with no resulting detached buffer.
- Transfer of Memory backed by ArrayBuffers is disallowed.

This specification attempts to be well-integrated with HTML by defining
n terms of the serialization behavior of Memory as a platform object, and
using the sub-serialization of the underlying SharedArrayBuffer to check
for transferring within an agent cluster, and not to IndexedDB.

Based on the earlier PR at WebAssembly/design#1074
dhil pushed a commit to dhil/webassembly-spec that referenced this issue Mar 2, 2023
Following the discussions in WebAssembly#90, it seems desirable and less
error-prone to make `rethrow` and `br_on_exn` trap in case the value on
top of the stack is of `nullref` type.

Closes WebAssembly#90.
dhil pushed a commit to dhil/webassembly-spec that referenced this issue Mar 2, 2023
In WebAssembly#90 it was decided to move the event section between memory section and global section. This change is reflected in the next paragraph, but not in the introduction.
dhil pushed a commit to dhil/webassembly-spec that referenced this issue Mar 2, 2023
Separate subsumption from instr sequencing; principal typing
dhil pushed a commit to dhil/webassembly-spec that referenced this issue Oct 3, 2023
dhil pushed a commit to dhil/webassembly-spec that referenced this issue Oct 21, 2024
Merge with WebAssembly/spec branch `wasm-3.0`
rossberg pushed a commit that referenced this issue Nov 6, 2024
* Rename "index type" to "address type" across the spec

* Update "index type" to "address type" in the spec interpreter

* Update section underlines

It is very important that you have precisely the correct number of dots.
It would be too much work for the computer otherwise.

* Rename "address type" to "addr type" in the spec interpreter

* Revert one stray interpreter update
rossberg pushed a commit that referenced this issue Nov 6, 2024
"index" was renamed to "address". Without this change, all JS tests
generated from the core tests will fail.
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

4 participants