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

MVP compatibility #5

Open
kmiller68 opened this issue Sep 17, 2019 · 11 comments
Open

MVP compatibility #5

kmiller68 opened this issue Sep 17, 2019 · 11 comments

Comments

@kmiller68
Copy link

It seems like there is a lot of value in maintaining MVP compatibly as long as it's not overly burdensome. In particular I have one possible version that would make it doable. I'll give an example then explain it below:

(custom "conditional-section" (target code (count 120)) (count 3) (replace 3) (replace 45) (replace 100)
(code (count 3)
(func ...) ;; should this match the signature index expected for function index 3?
(func ...) ;; should this match the signature index expected for function index 45?
(func ...) ;; should this match the signature index expected for function index 100?
)
(code (count 120)
(func ...)
...
)

Since the conditional section comes before the normal section, when parsing the normal section a feature detection compatible will just ignore section indices it has already replaced. We may want to still validate them but that's a different question. Additionally, since conditional sections come first we probably want to have them predeclare the total number of indices in that section.

Forward declaring the section has the nice benefit of also making the default value obvious, it's just what was in the OG section.

Thoughts?

@lukewagner
Copy link
Member

MVP compatibility could alternatively be achieved using a JS polyfill that splices a stream of bytes (coming out of fetch() and feeding into instantiateStreaming()). Because of the simplistic section nature, I think such a polyfill should be relatively small and not terribly inefficient.

@kmiller68
Copy link
Author

@lukewagner I dunno if I would really call that compatibility :P

Are there any benefits of the current proposal over this one? I can't think of any off the top of my head.

This proposal has the benefit of forward declaring the total size of the any conditionally compiled section so the consumer can allocate all the space it needs up front. It also forces some default implementation (even if it's unreachable) so you can't really get a totally malformed module.

@rossberg
Copy link
Member

@kmiller68, I think the current proposal is both simpler and more general. For example, yours seems to be assuming that you only ever want to swap out one function for another, but there is no reason to assume that that is sufficient in general -- for example, different variants may require completely unrelated sets of auxiliary definitions that you'll need to splice in.

This really is a very simple and generic mechanism for conditional compilation, which can have other applications besides the narrower use case of VM feature testing. On the long run, that is more attractive than short-term MVP compatibility IMO. Especially since this is fairly easy to implement in an engine, so we shouldn't wait for long before that becomes irrelevant.

@kmiller68
Copy link
Author

@rossberg Maybe I'm missing something, but I don't see how it's more general. I just gave the code section as an example but the above system could apply to any other section. What is something that's not doable in this variant?

I guess there's an advantage that it's easier on a producer to just splat out all the duplicate sections. That comes at the downside that it's slower on the consumer side to grow its internal buffers every time it sees a new section... IMO, it's probably more important for the consumer to be able to consume the module quickly.

@tlively
Copy link
Member

tlively commented Sep 17, 2019

I think it would be reasonable to have to forward declare index space sizes. There is some precedent for this in the bulk memory proposal's data count section, although that was necessary to enable single-pass validation while forward declarations in this proposal would just be for speed.

@rossberg Do you have a concrete use case that would require different index spaces for different features? My users on the C/C++ side would be happy with the ability to swap out functions one-to-one with identical type signatures, since that's all the GCC/Clang function multiversioning feature currently allows for.

@rossberg
Copy link
Member

@kmiller68, as I read your proposal, it doesn't allow e.g. adding functions conditionally, can I? I expect that to be a common case, because e.g. some set of replacement functions might involve additional helpers.

I can see how your proposal could be relaxed in that direction. But that makes it yet more complex. It seems much easier and more standard to just have conditional compilation rather than "patching" plus extending of code.

@tlively
Copy link
Member

tlively commented Oct 21, 2019

This issue is important to resolve because it informs the entire design of this proposal. Since we didn't have time for discussion at the last CG meeting where we talked about this proposal, I will add an agenda item for a future meeting so we can get wider feedback on how the community views the trade off between MVP compatibility and complexity.

@tlively
Copy link
Member

tlively commented Oct 29, 2019

We discussed this at the CG meeting today. Unfortunately @kmiller68 was not able to join and represent his position during the meeting, so I am certainly open to further discussion.

In the discussion I briefly recapped the proposal's goals and its design as written and pointed out that the goal of cross-engine portability is not currently achieved for MVP engines. I explained that it would be possible to support MVP compatibility if we used custom sections and that there is precedent for standardizing custom sections but not for standardizing semantically-meaningful custom sections. The question I posed to the CG was whether anyone felt that in the medium- or long-term it would be important for the ecosystem to continue supporting MVP engines.

The general consensus among the present CG members was that it will not be important to support MVP engines in the future, and that all engines should be expected to implement all parts of the specification with the exception of certain large features. Native threads and GC were specifically called out as being optional features. No one presented arguments in favor of supporting MVP engines.

One point that was not explicitly called out was that it is possible that this particular proposal should be exceptional among proposals and support MVP because the whole point of the proposal is cross-engine compatibility. But again, no one argued that supporting MVP is desirable at all.

@rcombs
Copy link

rcombs commented Jan 14, 2020

Thought I'd bring some library-dev perspective here:

  • Some form of compatibility with existing implementations is very desirable; otherwise out-of-band multiple-builds fallback becomes necessary, which kind of defeats the purpose of this proposal to some extent. My use-case will likely require me to support MVP engines for up to several years.
  • If a section-selecting polyfill as suggested by @lukewagner can be implemented efficiently in JS (and it sounds like it can be), that's probably good enough, with a few caveats:
    • In some cases, optimized versions of functions aren't appropriate for all input data, and the fallback version has to remain available at runtime. Additionally, the ability to switch between variants at runtime within the context of an implementation that supports all extensions allows the variants' behavior to be validated against each other (see e.g. ffmpeg's checkasm facility). This means:
      • I need the ability to add functions and data symbols without replacing existing ones. This could theoretically be dealt with by providing default trapping "variants" of all symbols to be added, but that adds a bit of developer friction unless tooling is updated to do so automatically.
      • I need the ability to detect feature availability at runtime, so I can explicitly select the optimized version of a routine in C code rather than expect it to simply replace the existing version. This should be achievable by simply providing 2 conditional versions of e.g. a hasSIMD128() function that just return 0 and 1, but it'd be helpful if those could be provided by build tooling (out-of-scope for this repo, I know, but seemed worth noting).
  • The text format needs to be extended to support placing functions in conditional sections.
  • In order for the above-discussed runtime-selection mechanisms to work, it's often necessary to reference a function that may not be runnable in a function that will run, on a branch that will not be taken. For instance:
if (hasSIMD128())
  vtable.additive_blend = additive_blend_simd128;

This could be handled by generating trapping versions of any functions defined only in conditional sections (as mentioned above), or by ensuring the implementation doesn't error at build-time on this kind of reference (as is done on e.g. macOS and iOS with weak imports).

I think most of these needs can ultimately be met by the current proposal (assuming that tooling develops further), but wanted to get all this on the record.

@tlively
Copy link
Member

tlively commented Jan 14, 2020

Thanks for these great notes, @rcombs! I think all your requests are addressable, details inline below.

In some cases, optimized versions of functions aren't appropriate for all input data, and the fallback version has to remain available at runtime.

This would be supported by making the fallback version a normal, unversioned function and adding a multiversioned wrapper function that determines whether to call the fallback or execute some other code path.

Additionally, the ability to switch between variants at runtime within the context of an implementation that supports all extensions allows the variants' behavior to be validated against each other (see e.g. ffmpeg's checkasm facility). This means:

I need the ability to add functions and data symbols without replacing existing ones. This could theoretically be dealt with by providing default trapping "variants" of all symbols to be added, but that adds a bit of developer friction unless tooling is updated to do so automatically.

I expect that most toolchains would only support generating multiversioned functions, not data. This is because LLVM at least already has facilities for multiversioned functions, but has no concept of versioned data. Multiversioned data could be emulated by having a multiversioned function to get a pointer to the data.

Although LLVM could in principle generate trapping variants of functions as necessary, I think it would be better overall UX if users had to explicitly opt in to that. Otherwise it would be possible to accidentally miss adding a variant and get mysterious traps on some subset of engine configurations, which sounds painful to debug. I'm not too worried about extra work for users, since using this feature will already require modifying code.

I need the ability to detect feature availability at runtime, so I can explicitly select the optimized version of a routine in C code rather than expect it to simply replace the existing version. This should be achievable by simply providing 2 conditional versions of e.g. a hasSIMD128() function that just return 0 and 1, but it'd be helpful if those could be provided by build tooling (out-of-scope for this repo, I know, but seemed worth noting).

Yes, this would probably be done with small multiversioned functions, as you suggest. They might be able to be placed in a standard intrinsics header, though, to prevent users from having to write them again in every project.

The text format needs to be extended to support placing functions in conditional sections.

Yep, this is something that we will need to figure out.

In order for the above-discussed runtime-selection mechanisms to work, it's often necessary to reference a function that may not be runnable in a function that will run, on a branch that will not be taken. For instance:

if (hasSIMD128())
  vtable.additive_blend = additive_blend_simd128;

This could be handled by generating trapping versions of any functions defined only in conditional sections (as mentioned above), or by ensuring the implementation doesn't error at build-time on this kind of reference (as is done on e.g. macOS and iOS with weak imports).

It's not decidable in general at validation time whether a particular function will be called or not, so this would have to be done with a trapping stub version of the function.

@rcombs
Copy link

rcombs commented Jan 14, 2020

Thanks for the detailed reply! Got some additional clarifications here, but your ideas seem to handle most of what I need.

This would be supported by making the fallback version a normal, unversioned function and adding a multiversioned wrapper function that determines whether to call the fallback or execute some other code path.

This is fine in most cases, but in others (where you know if you're going to need the fallback ahead-of-time and will be calling the chosen variant repeatedly), you want to swap out a pointer in a vtable to avoid having to do a conditional branch every time. As long as both symbols are available (as would be the case if I made the fallback unconditional and provided a trapping default for the optimized one) this should be fine, though.

I expect that most toolchains would only support generating multiversioned functions, not data. This is because LLVM at least already has facilities for multiversioned functions, but has no concept of versioned data. Multiversioned data could be emulated by having a multiversioned function to get a pointer to the data.

Hmm, so the use-case here is that an added (or replaced) function might need to reference data that the fallback version doesn't. I suppose that kind of data could just be there all the time (potentially unused), though? Shouldn't really be any need for that kind of thing to be conditional.

Otherwise it would be possible to accidentally miss adding a variant and get mysterious traps on some subset of engine configurations, which sounds painful to debug.

This is by and large a solved problem in the native world. In ffmpeg with checkasm, we run the vtable-setup function once for each available extension, toggling each one and validating output each time it returns a different function pointer. Additionally, we have assembler macros that prevent accidental use of, say, a 256-bit AVX instruction in a function intended for 128-bit SSE vector systems. This would be a very useful (optional?) tooling feature: only allow assembly of an extension instruction within a section that's conditional on that extension.

Yes, this would probably be done with small multiversioned functions, as you suggest. They might be able to be placed in a standard intrinsics header, though, to prevent users from having to write them again in every project.

Sounds perfect.

It's not decidable in general at validation time whether a particular function will be called or not, so this would have to be done with a trapping stub version of the function.

A trapping stub would work fine; in Mach-O, if you have a weak import to a symbol that isn't available and take its address (e.g. for a function pointer), you just get NULL, and I wasn't sure if I could expect that kind of behavior in wasm or not.

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

5 participants