Add GL_EXT_spirv_intrinsics_string extension#333
Conversation
Tobski
left a comment
Tobski
left a comment
There was a problem hiding this comment.
This looks good too - just a couple review comments and would request some more eyes and a draft impl before accepting.
| If GL_EXT_spirv_intrinsics_variadic is enabled, a literal string | ||
| argument may also match an unqualified variadic tail of a function | ||
| declaration qualified with `spirv_instruction`. In that case, an | ||
| OpString is also generated and its result <id> is appended as the | ||
| corresponding operand. It is a compile-time error for a literal string | ||
| to match a variadic tail with a `spirv_by_reference` or `spirv_literal` | ||
| qualifier. |
There was a problem hiding this comment.
I would suggest that the tail has to be qualified with spirv_string to make this work - non-string arguments ignore it, but strings will be converted to OpString arguments.
The rationale for this is that otherwise you are making concrete, with no get out, how strings should be passed to intrinsics, and I'd like to leave the door open to passing them as byte arrays in future.
By requiring spirv_string here, it leaves the door open for strings-as-byte-arrays to be passed as arguments in future.
There was a problem hiding this comment.
I guess spirv_string isn't a qualifier per say... so maybe not exactly that? But I do think there should be something to distinguish this behavior.
There was a problem hiding this comment.
Oof, that's a good point... spirv_string isn't a qualifier as you say (and it would probably create parser ambiguity to introduce it as one here), but some way to say "if there is a string, encode it as OpString" sounds like it would be necessary... I will think about this.
| void debugPrintfEXT(spirv_string format, ...); | ||
|
|
||
| void main() | ||
| { | ||
| float x = 1.0; | ||
| debugPrintfEXT("x = %f\n", x); | ||
| } |
There was a problem hiding this comment.
I think it would be illustrative to have an example with a string passed in the variadic tail.
There was a problem hiding this comment.
Sure. I could modify the example to add an "%s" argument to printf.
2 participants
This is the second half of #331, adding the string part (see #332 for the variadic extension). They are separate extensions because they describe orthogonal features with minimal interaction. The glslang implementation will follow soon.
This extension builds on
GL_EXT_spirv_intrinsicsand adds aspirv_stringpseudo-type that is only valid as a formal parameter type in aspirv_instruction-qualified function declaration. It furthermore provides the ability for literal string arguments to be passed to those parameters, as well as to variadic tails (when the variadic extension is enabled). It is lowered to an OpString whose result id is used as the operand.The main conundrum I've had to work through was determining whether literal strings are treated as primary expressions (which matches what glslang implements, eg. allowing a parenthesized
("foo")), or can only be used as function call arguments (which matches what can actually be derived from the motivatingGL_EXT_debug_printfextension, though it is a bit light on the actual grammar).I have deliberately decided to stick with the most restrictive reading of what is actually specified, since doing otherwise would ask this extension to do much more work to more generally define strings as a type and expression. I deemed that to be out of scope for an extension that just aims to represent functions like
debugPrintfEXTas aspirv_instruction. It may be worth revisiting this in a future extension that reusably and more generally defines string expressions in GLSL, or the referenced extensions should be revised to define the grammar in more detail and this one changed to match.