search

gpuweb / gpuweb

Where the GPU for the Web work happens!
https://webgpu.io
Other
4.79k stars 317 forks source link

should WGSL allow shadowing of builtins at module scope #2941

Closed dneto0 closed 2 years ago

dneto0 commented 2 years ago

WGSL does not have a module system.

So composing program fragments to make a big shader means textually concatenating shader text.

Suppose you have program parts A and B, and both of them use a builtin function XYZ. The program works as intended.

Now suppose B' is like B but provides its own definition of builtin XYZ. We assume the author of B' is fully aware of what they're doing, and makes B' work the way they want. But when making WGSL program A+B', the meaning of A has changed, silently. This is bad.

Why doesn't this problem occur with other module-scope declarations?

What happens when we expand the language? If we add a new type like bf32 (made up), or a new builtin (or a new overload of a builtin) then you have to enable it with an enable directive. So anybody combining an A' that uses bf32 sees the directive at the top advertising that bf32 is a newly introduced thing that this module uses, e.g. enable bf32_feature;.

If B has its own definition of bf32, then B can come with a warning saying "Sorry, this B program text is incompatible with enable bf32_feature". If bf32's definition is really for internal use within B, then B can be adapted over time to rename that builtin to again become compatible with enable bf32_feature.

If WGSL had a module system, then you might be able to relax things: A module author can declare the intent about whether a module-scope name is intended to be exported or not, i.e. whether its for users of the module, or for intra-module use only.

dneto0 commented 2 years ago

cc: @ben-clayton @dj2

ben-clayton commented 2 years ago

I'm afraid I don't feel this argument is particularly compelling, and heads in a direction I hope we'd avoid.

Programs composed of multiple chunks of text are still likely to be maintained by the same project and developers, and I personally believe this 'silent change in behaviour' is unlikely to be a real problem - or at least an issue that wouldn't take a developer a few minutes of investigation to figure out and fix.

@kainino0x had proposed some time ago that WGSL could be a "living language", in that new language features (which are not dependent on GPU hardware capabilities), could be added over time, without the need of enable directives, so long as the language remains backwards compatible. At the time Kai proposed this, I was concerned about offline tooling not being able to recognise what WGSL version it was processing - but after consideration, there are plenty of existing languages / compilers that work just fine with this model (Golang being one). As I believe Kai previously pointed out, this "living language" model has already been successfully used by JavaScript, so we'd not be breaking new ground here.

The alternative to the "living language" is explicitly enabling each and every new language feature with an enable directive, and each implementation having to maintain a separate codepath for each enabled / disabled feature. This approach doesn't seem scalable, and adds a whole bunch of complexity for implementers and our web developers.

Allowing builtins to be shadowed at module scope is the only way we'd be able to maintain backwards compatibility without forcing each feature to be explicitly enabled. For these reasons, I'd really like to see shadowing of builtins to be allowed, which could be spec'd easily by stating that builtins exist in a scope one higher than the module scope.

With all this said, what we have right now can be relaxed later to allow shadowing of builtins, so there's no need to change anything for V1. I just wanted to make clear that I think we should seriously consider the alternatives to requiring explicit enables.

dj2 commented 2 years ago

Just to be clear, this would be shadowing of builtin functions and builtin types to allow expansion of types in the future as well?

So, I'd be able to do type i32 = f32; if I really wanted?

ben-clayton commented 2 years ago

So, I'd be able to do type i32 = f32; if I really wanted?

I wouldn't propose we explicitly forbid it in the spec, but as it currently stands i32 is a keyword and so no, you wouldn't be able to type this, even if we enabled shadowing.

You raise a good point though - the "living language" wouldn't be able to introduce any new keywords. New types, etc, would all have to be identifiers. For consistency, if we were to go down this path, maybe i32 and friends should be identifiers too.

dneto0 commented 2 years ago

The alternative to the "living language" is explicitly enabling each and every new language feature with an enable directive, and each implementation having to maintain a separate codepath for each enabled / disabled feature. This approach doesn't seem scalable, and adds a whole bunch of complexity for implementers and our web developers.

I disagree. Every once in a while you make a new enable that batches together old individual things.
Example: enable wgsl2024; pulls together all desired core features. Make a new one every couple of years, or whatever epoch is desired.

If tooling projects think there is too much complexity in maintaining old fine-grain subdivisions, then they can move to a new epoch, and acts as if wgsl2024 is the baseline.
This is a new language dialect, but formalizes the "living language" idea.

This would be much like how Rust evolves (apparently). https://github.com/rust-lang/rfcs/blob/master/text/2052-epochs.md (modulo the discussion of deprecations).

Analogy. This is valid K&R C:

#include <stdio.h>
foo(a)
int a;
{
  return 2* a;
}

main() {
  return foo(2);
}

Are you sure your toolchain supports it? It so happens that clang on my machine does. But I was surprised by it.

I expect that it's kind of understood that a C compiler you get today might not support K&R C. At some point it would be understandable that a C++ compiler does not support C++1998 that is not also valid C++11. At some point it would be understandable that the system C++ compiler would understand C++11 without having to ask nicely (-std=c++11).

What I'm proposing is that formally, you always need an 'enable' for a new feature that isn't backward compatible.
But new epochs make new language dialects, and over time "all" the tools and browsers might care about will only implement the newer epoch dialects.

dj2 commented 2 years ago

That doesn't work with the web though. You'll always have pages which enable foo;, enable bar; even after you have enable wgsl.2030; which includes those two things. So, we always have to check and support the individual enables else we can break existing content.

So, for each enable we'll have to have the code that supports it or not and we can't assume it won't be used after a certain amount of time.

kainino0x commented 2 years ago

I think the problem of adding new builtins to the prelude can be sidestepped by just not doing that. WGSL could reserve std as an identifier and then add new things as std::newThing, or use a sigil for new builtins such that they can't collide with user code (__newThing or ::newThing). Keywords are a different thing, but Rust for example has managed pretty well adding new keywords only "contextually" (they don't need to be reserved because they can't grammatically appear where identifiers would appear).

You can still do feature packs like enable wgsl2024 that dump buckets of things unprefixed into the prelude, but it still has maintenance cost and I don't think it's going to be necessary for a long time if ever.

(I'd be almost tempted to suggest that all builtin identifiers that aren't reserved words have some kind of sigil or convention that makes them visually distinct from user defined stuff, but that's most likely overkill.)

ben-clayton commented 2 years ago

If tooling projects think there is too much complexity in maintaining old fine-grain subdivisions, then they can move to a new epoch, and acts as if wgsl2024 is the baseline. This is a new language dialect, but formalizes the "living language" idea.

When is an epoch? Whatever number you say, there will be shaders on the web that will be not be using the latest enable pack. The moment you decide to update your baseline, you have the symbol collision problem, and you've broken existing shaders.

How would you motivate developers to start their new shaders with enable wgsl1234 ? If V1 contains all the features I need, I'm not going to type additional, unnecessary stuff. So I'd bet that most WGSL would be baseline.

That doesn't work with the web though. You'll always have pages which enable foo;, enable bar; even after you have enable wgsl.2030; which includes those two things. So, we always have to check and support the individual enables else we can break existing content.

Completely agree with @dj2 here.

I think the problem of adding new builtins to the prelude can be sidestepped by just not doing that. WGSL could reserve std as an identifier and then add new things as std::newThing, or use a sigil for new builtins such that they can't collide with user code (__newThing or ::newThing).

Ensuring that all new identifiers get placed in a new namespace would work, but:

Restating my initial point: allowing shadowing of builtins / types would allow us to add new things to the language and avoid a forever expanding set of enables. The concern about code snippet joining and 'silent change in behaviour' is a trip hazard, but we have no evidence on how common or annoying this would be. I believe maintaining a sea of enables (by both implementor and user) would be more annoyance than this theoretical issue.

dj2 commented 2 years ago

Concretely what would that mean for the spec:

As the language grows, we add new types and builtins without enables entries. Warnings may be added to implementations when a shadow of a builtin/type are found.

dj2 commented 2 years ago

@ben-clayton points out for the shadow type context question, the answer is probably no. This would get very confusing with a type constructor if the user declared an i32 function, we could no longer have context for what to call.

(Also, type aliases and structs could be problematic here)

It's also more consistent to treat shadows all the same, so the type would not be recognized as such until the shadow goes out of scope.

dneto0 commented 2 years ago

Changing to non-editorial, to address discussion about i16 and i64 in #2983

dneto0 commented 2 years ago

In the 2022-06-07 meeting we uncovered the same discrepancy about how/whether author's declarations at module scope can shadow predeclared items in WGSL.

dneto0 commented 2 years ago

Google has been discussing this internally, and think we first need to agree on how WGSL will evolve for "sugar" features. This addresses part of #600

Summary:

Proposal for smooth language evolution

Requirements:

Proposal details:

Discussion

Open questions

Next considerations:

kdashg commented 2 years ago
WGSL meeting minutes 2022-06-21 * DN: Larger conversation on language evolution and smooth monotonic. Divided features into sugar or not. Sugar is done on software side, rolled out by all browsers at same time (e.g. do {} while). What we want is monotonically increasing set of sugar features. There is backwards compat because they’re added. Then evolve language with minimal tuling support. No a big set of on/off features, just outer shell that’s increasing. Then if you ever have breaking change call it WGSL2. (Basically semver). Want devs to opt into higher shells for functionality. Big debat, in shader with declaration of some form, or API side saying I want to use WGSL features up to 1.55. So, tooling can be simple and implement a single level and understand everything below and when going through code, report the maximum minor version used (you used up to 52) and all good. Then on the API side can say use up to 55 so you don’t get accidental feature creep. Can set version to what is know to be supported in all browsers. Framework to think about how features evolve without having to add things that require namespaces and complexity. Then debate is in vs out of band. Once that’s resolved we can then say do we allow a user declaration at module scope to shadow a builtin that maybe introduced? Would like understanding about monotonic increasing and backwards compat features. At least start conversation. * MM: Not sure if can sign off on this immediately because 1) from dev point of view if there is no technical reason why one feature depends on another it’s hard to say i want to do feature x but i have to do y first. From a browser dev point of view 2) this is not how javascript is developed and JS arguably has 2 versions with use strict and think much of the js community sees that as unfortunate. * AB: To the first point, choosing only to implement some features is bad for the language. For c++ standards, you may have experimental support as things get added but you don’t say you do c++17 if you skip some of the features. Think rust and go are similar. When you support the version you support all the features. We’re going to agree to all the features. We’re just not going to agree to features we won’t do. If there are things a browser doesn’t want they won’t be standard features. So, not a big deal. For js, won’t speak to that. * BC: To reinforce what AB said about 1). Would hope the WG would agree to features that make it into the next minor release. So, if things that are blocking like Google doesn’t want to do a feature of the point release they we shouldn’t have agreed to have in the release. So, like we're agreeing on v1 now, i’d foresee us with this approach we’d meet a YY interval and agree what goes in next minor release and agree we fell comfortable to implement it. * KG: This seems like something in this direction seems reasonable. Need to give more though. Important question is core one of what do we do with new builtins. If we make `determinant` do we use the user one as an override of the builtin, that seems tentatively where we were going. * MM: Discussing 2 different models. a) was proposed, every version is numbered and author declares what version they want and if they collide with stdlib then they’re doing it wrong and get compile error. b) is the language is not numbered and the sugar features are implemented whenever the browser gets around to it and strictly additive and don’t cause existing code to break and when new sugar features become implemented in the browsers then authors start using them. Authors start using at own schedule. Both are possible, c++ is example of first, JS is example of second. * KG: Not exclusive that way, can have increases that have bits that tag along anyway. Want to read in more depth. * BC: Make clear that proposal is not when you request a new feature request new stuff gets enabled. You don’t have new symbols appear because you requested later version. This would have 2^N compiler complexity of enabled/disabled features. Proposal is so compiler doesn’t know about target version, it just compiles and then reports high water mark and at shader level you can check. * JB: To js model, dom apis are introduced piecemeal but JS language is linear They have year numbers and you say which year snapshot yo have implemented. So the lang and standlibrary are separate things. * KG: **Think offline and come back next week**.
kdashg commented 2 years ago

I see there as tension between:

What I think might work well is to have a slightly more sophisticated identifier resolution. What if you could do std.u64 to access some (future!) u64 type directly, but that a bare u64 would preferentially resolve to within the scope of the module, so to e.g. the struct u64 polyfill above.

Conceptually I imagine this as having a namespace std, and using namespace std being implicit, and further, that u64 (_.u64?) would resolve preferentially to an in-scope declaration, and then secondarily to any of the "used namespaces" of the scope. 

namespace std {
[...]
type fvec4 = vec4<f32>; // e.g.
}

namespace _ { // module-scope
using namespace std;
// Begin WGSL module
struct u64 { lo: u32, hi: u32 };
fn less(a: u64, b: u64) -> bool {
  if a.hi < b.hi { return true; }
  if a.lo < b.lo { return true; }
  return false;
}
fn min(a: u64, b: u64) -> u64 {
  if less(a, b) {
    return a;
  }
  return b;
}
[...]
// From some copy&pasted code:
fn foo(a: std.u64) {
  [...]
}
// End WGSL module
}

You could even imagine that we do something like:

namespace std22 {
[...]
type fvec4 = vec4<f32>; // e.g.
}
namespace std23 {
using namespace std22;
[...]
type u64;
}
namespace std {
using namespace std23;
}

Thus

let _: fvec4; // easy
let _: std.fvec4; // safe
let _: std22.fvec4; // precisely
let _: std23.fvec4; // technically

let _: u64; // easy
let _: std.u64; // safe
let _: std22.u64; // ERROR!
let _: std23.u64; // precisely
kdashg commented 2 years ago
WGSL meeting minutes 2022-06-28 * KG: Maybe just coming back around to namespase don’t seem like a bad idea. Was worried about them for a while. Main new thing here is idea of having builtins dumped in global namespace by default but could passively overwrite them with user functions and would pickup user function but if you wanted to you could ask for the old builtin by name, deliberately specify std.min() and get both worlds. * MM: Interesting,as that was what DG said internally. * GR: If we redesigned HLSL we’d probably do this as well. So think it’s a good idea * MM: To add namespaces? * GR: To add namespaces for buildings * KG: And use it by default? * GR: Yea, just adding my +1. * KG: Other things for ideas, was idea of explicit versions for std library builtins. So actual releases could have std.22 from year 22 and next year have std.23 including std.22 + more. So, could get explicit and say only initial release things. Also talked about compiler warning, like linting task you’re using builtin you didn't’ qualify. Opt-in warning that some may like, but probably not worth an error or enable. * MM: Bunch of different topics here. One of views we have that is strong is we don’t want the website to tell the browser i’m using a particular version of the compiler so please don’t expose anything not in this version of the compiler. GLSL did it this way, if you said 410, then things in 420 were not to exist. Feel strongly we don’t want that design as it requires n versions of the compiler. For things that are not breaking it’s not worth it. * DN: Saw BC and KG nodding in agreement * BC: main thing, don’t want compiler to know every version. Main thing I don’t want is maintaining a full history of everything added and having to test with everything enabled/ disabled. Avoid the 2^n problem with testing. What is apples opinion of a high water mark to prevent accidental usage of bleeding edge features * MM: Didn’t understand so no position * BC: Imagine webgpu api and compiler are separate components. Mental model is you have WGSL given to compiler and there is a minimal amount of effort in compiler to maintain a feature used to highest version watermark. For any given feature used in WGSL we maintain the highest version. * DS: For any feature X, compiler knows “X was introduced in minor version 23” for example. And as the compiler encounters feature X in WGSL source, it remembers “23 was used”. And it takes a max over all those minor numbers. Then in the end the compiler reports the highest minor number N encountered. * BC: Compiler always builds at it’s highest level and just reports what highest level it found was. So don’t have to remember everything * GR: This is what HLSL compiler does. * AB: MOtivation is that new sugar that we release it rolls out to browser at different speed. So, you opt-in so you know it’s guaranteed it’s going to work everywhere and can transition when I want too * MM: You don’t opt-in to the version. Sounds like in this model the browser tells you want version you’re using and you can then do what you want with this knowledge. * AB: Could be done that way, but it comes down to we think there should be something on the API side where you specify the WGSL version and the default is 1.0. If you want more features you have to opt-in. Internally folks have been very firm, not adding features without people knowing. You have to opt-in. * MM: Confused now, as AB and DS seem to be saying different places. World of WGSL teling user what they used is different from browser saying * AB: If the api says allow highest 1.0 then wgsl says 2.0 it can issue an error * JB: Could do what MM heard by including watermark in compilation info. Then no enforcement and user decides what they want to do. Folks always say they want to target this SDK to get a handle on audience. Apple and Android APIS do that. * AB: We think it’s important that you opt-in to something more then the default. If you don’t do anything on API side you get 1.0 WGSL until you specif. How do you do this so it’s nice on tooling. Initially thought of GLSL way of version string in each shader but seems overkill. Can just say I’m going to use this version and it works everywhere. Don’t have to worry about it working on other platforms as know what they support * JB: So don’t want it on the user to verify they used too much? * DN: That’s correct. * MM: Don’t understand what GLSL does and what’s proposed? In one author says i”m targeting this version of the language but saying in the API instead of language * BC: From end user pov they seem similar. But implementation side by enforcing the shader only gives high water mark, only as an output, not input, guarantee backwards compat with the language. Everything compiled before must continue but we track must advanced feature used. Don’t think the justification has been prosperity explained, the user always specifying the language feature allowed is to prevent someone from writing a shader that is only supported by FF and doesn’t work on other browsers as it doesn't’ work on other browser yet. A seat-belt to make sure they don’t opt-in to to high things * GR: Very similar to HLSL. You say SM6.6 and the compiler tracks an dif you use something higher compiler fails with error. To show how easy it is to use a feature that isn't’ supported talking about things like passing existing types to existing builtin functions. We could add in future versions and it gets used accidentally. * MM: Want to contrast with JS as last week but got cut off. JB said JS is versioned. Not quite what getting at. If you use ES2016 you can’t ask. The only version enforced is `use strict`. ES2016 isn’t enforced you don’t ask for it. So the JS model is pretty good. * BC: You could if we did expose in the API give me the most recent version of WGSL support and they could feed it back into as the requested version. Goes against most of what’s asked for, but you could do it. Also increases fingerprinting I think * KG: Not worried about that * MM: Already observable. * KG: No more being exposed. It’s a conscious choice to always just use the things. Only thing for feedback, would prefer if it was in-band vs out of band. Find having to hold shader source to representative shader and a tuple (src, version) don’t like having to do that and would prefer if it was a src file annotation. * AB: Some folks internally who prefer that as well. Not sure if that’s a GL background thing. It then needs to be in every shader. Some advantages like copying code off web and know everything about it. If generating tonnes of shaders lots of duplication, vs just appearing in the API. * BC: Big games/projects have 1000’s of shaders and moving to more recent shader do you force everyone to update their shaders? If it’s generated, does it matter if it’s in the shader or the API? * JB: Not sold on markup in shader text. Based on plan knowing the version isn’t necessary to interpret the source. You know what the src means without know what version it’s marked as. You only report high watermark and verify it’s what is expected. * KG: Come back to this next week.
litherum commented 2 years ago

RESOLVED: Should WGSL allow shadowing of builtins [both types and functions] at module scope? Yes.

kdashg commented 2 years ago
WGSL meeting minutes 2022-07-05 * DN: Read Kelsey’s comment from last time, and seems reasonable to me (independent of others at Google). Would be compatible with allowing shadowing at module scope. * AB: Concern we’d have to specify all the namespaces for that idea. Independently there are many at Google internally who like the idea of the idea of the issue: allowing user-declared module-scope declarations to shadow predeclared language things. * MM: Thought we were going to talk about language evolution. Happy to accept allowing shadowing. * **_Resolve: Allow user-declarations at module scope to shadow predeclared things._** * KG: Think that shadowing thing is forward compatible with namespaces. * DN: Where do we stop: reserved words, types, var, etc.? * JB: Scheme is super flexible. But draw a line in the sand. * AB: Think we can stop reserving types. * MM: Types and functions are reasonable to allow, because users can make their own. But they can’t make their own var, while, etc. * JB: We already have problems at parse time because we need to forward-resolve function names and types. * DN: **So I’ll post PRs 1. Allow the module-scope shadowing, and 2. Pares down the reserved word list, removing items that look like types and functions.** Keep only things that look like what would be a keyword (control flow, decl specifier, etc.) in WGSL.
dneto0 commented 2 years ago

I forked the language evolution / versioning topic out to #3149 so I could mark this as "Resolved - needs spec"