Skip to content
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

Is there a way to make i31 shared? #57

Open
tlively opened this issue May 2, 2024 · 5 comments
Open

Is there a way to make i31 shared? #57

tlively opened this issue May 2, 2024 · 5 comments

Comments

@tlively
Copy link
Member

tlively commented May 2, 2024

As @rossberg pointed out in #55 (comment), it would be nice if we could allow i31ref in shared contexts. The trouble is that it would then have to be a subtype of both eqref and (shared eqref) (at least AFAICT), and we have never allowed a type to have multiple supertypes before, making this a risky new frontier. We should experiment with this and see whether we run into problems in practice.

@tlively
Copy link
Member Author

tlively commented Dec 5, 2024

we have never allowed a type to have multiple supertypes before, making this a risky new frontier. We should experiment with this and see whether we run into problems in practice

This is not quite true, since the bottom heap types all have multiple supertypes. This would be similar.

I had been thinking, inspired by #80, that we could type i31.new as [i32] -> [(ref (bot-share i31))], and have (bot-share i31) <: i31 and (bot-share i31) <: (shared i31), but really the original idea of just having i31 <: (shared eq) in addition to i31 <: eq is simpler.

@Liedtke
Copy link

Liedtke commented Dec 8, 2024

but really the original idea of just having i31 <: (shared eq) in addition to i31 <: eq is simpler.

So this would result in rules like intersection(shared eq, eq) == i31, correct?

[Paragraph removed as it was based on me misreading the previous comment's proposed subtype relationship.]

There had been discussions around br_on_cast and the explicit source type immediate and I think there were concerns from some toolchain that it might be difficult to provide a fitting source type and afair it was resolved with "well, you can always use anyref for anything in the any hierarchy". For i31s the question would be: "Should I use any or shared any as the most generic super type?"

I don't think any of that is simple or intuitive and it sounds to me that the only reason for these inconsistencies would be "We can re-use the existing ref.i31 instruction for shared i31refs instead of having to add a new ref.i31_shared".

The overview of this proposal lists 37 instructions and we'd reduce that number by 1. Having to implement decoding and typing for these two "create an i31" instructions will probably be significantly less work in any engine compared to the difficulties and issues that arise from having this one type that can "pierce through sharedness boundaries" like no other type can.

Building type-based optimizations that handle these i31 corner cases correctly doesn't sound easy and logic bugs in these optimizations are often an exploitable vulnerability.

@rossberg
Copy link
Member

rossberg commented Dec 9, 2024

@Liedtke, you may well be right that it isn't worth it. But note that it's not just for the sake of saving an instruction. It would also be in order to have the ability to pass on an i31 to where a shared ref is required, and vice versa, without having to untag and retag it redundantly. Though we could probably add specific type coercion instructions to cover that.

@Liedtke
Copy link

Liedtke commented Dec 9, 2024

[...] have the ability to pass on an i31 to where a shared ref is required

Do we have an idea why that could be needed?
So far my mental model was that any shared-everything application is going to make "everything" shared in their type system and then they wouldn't have many reasons to convert between shared and unshared types.

Similarly, I see two options:

  1. The input is i31: In this case the module needs to emit a (ref.cast (shared i31) myI31) as there wouldn't be subtyping between the i31 types.
  2. The input is something more generic, let's say eqref: The caller is going to need explicit conversions for null (i.e. creating a new ref.null (shared none)) and for anything else that could be in here. For structs and arrays this would require some complicated logic to "clone" the object structure in the shared space with cycle resolution etc. So probably this would be some sequence of br_on_null, br_on_cast (shared i31) followed by the more intricate cases and at that point I'm not sure if we gain much if the handling for i31 saves 4 wire bytes. (From a performance perspective for V8 we can add an optimization to eliminate redundant conversions to i32 and back if anyone is concerned about it. This would be much less of an issue than adding the complexity implied by the multi-inheritance.)

Is there a guarantee by the wasm standard that a ref i31 is not an allocated object?
If not, it would also be a bit surprising to have implicit conversions that create a runtime overhead (allocation + copy).

@rossberg
Copy link
Member

rossberg commented Dec 9, 2024

[...] have the ability to pass on an i31 to where a shared ref is required

Do we have an idea why that could be needed?

I don't have any concrete example, but I'd expect something like this could arise in a scenario where there is a distinction between global and thread-local state, while scalars are moved between the two. But admittedly, that is rather vague.

Is there a guarantee by the wasm standard that a ref i31 is not an allocated object? If not, it would also be a bit surprising to have implicit conversions that create a runtime overhead (allocation + copy).

The intent of i31 is to represent unboxed scalars, but of course, the spec has no way of enforcing that, since it can ultimately only specify observable behaviour. But yes, if such a coercion would run the risk of requiring allocation under some common implementation schemes, then it should not be provided.

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

3 participants