Conversation
When spirv_instruction specified an extended instruction set, operands were passed to createBuiltinCall as ids, so a spirv_literal parameter became id 0 instead of its literal value. This asserted in debug builds and silently produced bad SPIR-V in release builds. Translate spirv_literal operands before choosing between OpExtInst and the plain opcode, and add a createBuiltinCall overload that takes IdImmediate operands, as createOp already does. Fixes KhronosGroup#4317
|
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4317.
When
spirv_instructionnames an extended instruction set (set = "..."), the operands were emitted throughcreateBuiltinCallas ids, skipping thespirv_literalhandling used for plain opcodes. A literal parameter became id 0, which triggered theaddIdOperandassertion in debug builds and produced invalidOpExtInstoperands in release builds.This translates
spirv_literaloperands before choosing betweenOpExtInstand the plain opcode (in both the unary and aggregate paths), and adds acreateBuiltinCalloverload that takesIdImmediateoperands, ascreateOpalready does.The new test
spv.intrinsicsSpirvInstructionSetLiteral.compcovers both paths with OpenCL.stdvloadnand OpenCL.DebugInfo.100DebugOperation; spirv-dis shows the literals encoded correctly (vloadn ... 2,DebugOperation Swap). The module can't pass spirv-val:vloadnneeds a physical addressing model and DebugInfo instructions must be at global scope, and no instruction set lets GLSL use a literal operand legally inside a function body. So the result is added tovalidation_fails.txt.