I've been working a lot with #glsl for a while now, writing complex code. Overall, I feel like the single improvement that would make the most difference for the language would be support for pass-by-reference instead of copy in/copy out syntax, as suggested here https://github.com/KhronosGroup/GLSL/issues/84
This would make it possible to do a lot of things that are either not possible or ridiculously inefficient to compile, including object oriented programming.
Also one thing that should be easier to do (I imagine) if pass by reference exists, is to accept arbitrary length arrays as parameters.
Right now you can declare a local array and iterate on it with a for loop on its .length(), but you can't move that for loop to a common function that accepts various array sizes as input. The only way to avoid repeating the code is using macros, which sucks.
Again for a language that inlines everything this seems like a strange omission.
@julienbarnoin I think in modern day, it's a perfectly reasonable ask. The one place it gets a little funky is that references add the potential problem of aliasing. If you tried to write an extension and add it to GLSLang, I think that's likely the thing that would get brought up as a problem.
That said, if we added a byref qualifier to the out, in, inout list, I think we could also add a restriction that no two ref parameters can be passed the same variable.
@julienbarnoin There are already references in the language, sort-of. The interpolateAt things are functions that take an input variable but no copy happens before the invocaiton of the function. I'm not sure how that's worded in the spec but I imagine we could do something simpler for byref
@julienbarnoin Really, I think what's missing is someone to formalize it into a GL extension and then implement it in GLSLang. We don't really need any other implementations if we just expect it to be used to target Vulkan.
If you want it in GL or GLES, you'd have to talk vendors into providing it. I don't think wiring it up in Mesa would be too bad.
Unfortunately, no one's really pushing GLSL forwards these days so just filing an issue isn't likely to do much. 😕
@gfxstrand@julienbarnoin If someone wants to implement this in glslang I would definitely consider the patch, and if you write up a GLSL extension spec that would probably help too. In terms of nobody moving GLSL forward, kind of true but also nobody is holding it back, if you write an extension spec and a glslang patch, I'll merge it.
@gfxstrand@julienbarnoin I have been trying to brainstorm GLSL improvements and pass by reference is definitely on that list. Maybe also some kind of generics (more like Rust traits than C++ templates). And really we need a GLSL users' group to help move the language forward.
@crzwdjk@gfxstrand Just curious if you don't mind, is this a list of desirable GLSL improvements that you think would be worth merging if a third party implements them? Or are you going to be actively working on some of these yourself?
@julienbarnoin@gfxstrand I can't really actively work on too much improvement, we're only funded to to maintenance. But if you take a look at the issues in the GLSL repo, that would give you a good idea of what people want. In addition to the stuff already mentioned here, I think a separate compilation model and eventually language interop would be useful (and pass by reference would help there too).
@crzwdjk@gfxstrand Well, it's just a start, but I have a quick and dirty proof of concept that works for local variables. Of course now getting it to specialize for storage buffers will be the hard part, and I'm sure there's more that can go wrong, but so far so good.
@crzwdjk@gfxstrand Just as you said though, the function will need to be specialized for Storage Buffers. Probably not easy.
The function is expecting a %_ptr_Function_float and I'm just naively passing it my variable which is a %_ptr_Uniform_float in this case.
As a result when executed I get a Validation error: vkCreateShaderModule(): Error during shader instrumentation in spirv-opt: line 47: OpFunctionCall Argument <id> '30[%30]'s type does not match Function <id> '7[%7]'s parameter type.
@crzwdjk@gfxstrand Yeah I looked into it a bit and don't see a trivial way to use existing functionality to do this so far so it'll take actual work. I won't dive into it too much for now as I need to move on other things too, but I'll keep thinking about it and trying things once in a while.
I saw there was already cases that trigger "pass-by-original" in the code, which is pretty much what I want I think, but only works if the storage class matches there too.
@crzwdjk@gfxstrand
Spent a bit more time looking into this, I looked through the code a bit to determine at which level specialization could take place.
Getting functions to execute with original variables implies recursively verifying the calling argument qualifiers are suitable for use in the function at each call site, so we don't discard a const etc.
These conditions are checked during parsing so we can't easily defer this and post process the CFG or do it during spirv generation.
@crzwdjk@gfxstrand I think handleFunctionCall should be the appropriate place to instantiate a specialized version of a function. This means we need to have the function definition parsed by then, so calls to forward declared but not defined functions that take byref arguments would not be allowed - like with C++ templates. A bit annoying but not the end of the world i think. Unlike C++ we could still handle these in multiple passes because there's no recursion but... it would be a pain.
@crzwdjk@gfxstrand It would also require more extensive function name mangling for byref parameters so we can instantiate with the exact qualifiers required. I'm not sure there's another way, it does highlight why copy in/copy out was an attractive simplification though. But... I'm still curious to see where this leads.
@julienbarnoin@crzwdjk I don't know if mangling is actually required. GLSL supports overloading, at least for internal intrinsic functions (it's like its one modern feature) and SPIR-V function names are just debug information. It might be a problem if linking two compiled shaders together but that's not usually a thing with GLSL.
@gfxstrand@crzwdjk there's definitely mangling happening internally in glslang, but it shouldn't be a limiting factor indeed as far as i can tell - though it's possible it's necessary to uniquely identify a function definition internally, not clear yet.
@gfxstrand@crzwdjk There's multiple difficulties on the way to get this working, especially for working on it casually as a side thing.
However I think this can be divided in steps that can inform the next one.
My first step would be to implement temporary qualifiers that explicitly specify a storage class, like byref_local, byref_uniform etc.
This part is simple and should help figure out what needs to work to do this automatically, and I could share the fork if someone wants to take a look.
@gfxstrand@crzwdjk There's a lot to the language so I think being able to play with it in this state will reveal blind spots too.
Also, it would already be useful in practice for me, I might even ship with it :)
@julienbarnoin@crzwdjk That certainly feels like a decent first step. It would make for an awkward extension but if it lets you prove out the concept and show that it really does improve shader perf in your engine/game, that's a really good indicator that cleaning it up and doing it "properly" is worth the effort.
@gfxstrand@crzwdjk Yeah I wouldn't want it published in that state either, but as a fork for people who want to give it a shot it could still be worthwhile, until we figure out something nicer.
@gfxstrand Oh that's interesting, I never thought about those (I haven't used them).
The spec says:
"For all of the interpolation functions, interpolant must be an input variable or an element of an input variable declared as an array. Component selection operators (e.g., .xy) may be used when specifying interpolant. Arrayed inputs can be indexed with general (nonuniform) integer expressions."
It's quite specific to interpolant inputs though so I imagine they would remain a special case.
@julienbarnoin Yes but I think you could make byref require that it be a variable dereference in a similar way.
The other thing that gets tricky is that we need to somehow ensure that we know the storage class. So you probably need to specify in the function prototype that it takes a reference to a UBO or similar. That or GLSLang will have to specialize under the hood.
I think GLSLang does specialize today, though so that may not be too hard. @crzwdjk may know.
@gfxstrand@crzwdjk Mh... ideally it would be able to determine this at the time it's being called and specialize the implementation that gets called accordingly, it would be a poor user experience to have to specify that upfront (especially if one wants to call it with parameters from multiple storage classes).
@gfxstrand By the way, any particular reason you were thinking of "byref" as the keyword for specifying this? I was thinking of simply "ref" like in C#. I imagine either should work, it's far from the main issue, just curious if you had a reason in mind.
And it also has a no aliasing restriction; you can't have two names for the same variable memory and have a RW, WR, or WW hazard in the same function. https://www.w3.org/TR/WGSL/#alias-analysis
And with all that our compiler does call site specialization until it boils those pointer params away. It's convenient for the programmer but can bloat code size (hopefully no more than hand rolling it yourself)
Add comment