Update SPIR-V to 1.4.1 #115
Conversation
| @@ -132,7 +132,7 @@ | |||
| { "kind" : "IdResult" }, | |||
| { "kind" : "IdRef", "name" : "'Set'" }, | |||
| { "kind" : "LiteralExtInstInteger", "name" : "'Instruction'" }, | |||
| { "kind" : "IdRef", "quantifier" : "*", "name" : "Operands" } | |||
| { "kind" : "IdRef", "quantifier" : "*", "name" : "'Operand 1', +\n'Operand 2', +\n..." } | |||
There was a problem hiding this comment.
is this really the name of an operand? We have some code that tries to make decisions based on that field.
There was a problem hiding this comment.
That's what's in upstream, so I suggest we patch up that code to make it easier to merge in the future.
There was a problem hiding this comment.
If it's a massive complication I can ping Khronos internally and see if we can change it back.
There was a problem hiding this comment.
I think we can get around by checking the Op itself instead of the argument name
There was a problem hiding this comment.
Yes. That field is used to generate the spec. So I'm not sure we can convince upstream to change it.
|
I think that's be the cleaner option then
…On Mon, 21 Oct 2019, 17:47 Dzmitry Malyshau, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In autogen/external/spirv.core.grammar.json
<#115 (comment)>:
> @@ -132,7 +132,7 @@
{ "kind" : "IdResult" },
{ "kind" : "IdRef", "name" : "'Set'" },
{ "kind" : "LiteralExtInstInteger", "name" : "'Instruction'" },
- { "kind" : "IdRef", "quantifier" : "*", "name" : "Operands" }
+ { "kind" : "IdRef", "quantifier" : "*", "name" : "'Operand 1', +\n'Operand 2', +\n..." }
I think we can get around by checking the Op itself instead of the
argument name
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#115?email_source=notifications&email_token=AAAMDOWFR2FI7JPMWYNZLE3QPXFJNA5CNFSM4JC6QOF2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCIUM4HQ#discussion_r337095461>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAMDOVZFRIYVPW42WNWSXDQPXFJNANCNFSM4JC6QOFQ>
.
|
|
I think I finished up the rest of the upgrade; I'm not sure if anything else needs to be done but I'd be willing to put in some more time if needed. |
|
It might make sense to add @antiagainst as a reviewer as well in this case. |
Some legwork for #108 somebody will still need to fix the submodule to point to KhronosGroup/SPIRV-Headers@c4f8f65