khronosgroup / spirv-tools Goto Github PK

The spvBinaryParse API isn't quite right:

1. It requires the client to translate endianness when the module being parsed is of a foreign endiannes. (e.g. I'm on a little-endian machine, but the module is big-endian). Frankly, most clients won't bother to write that code since the world seems dominated by little-endian machines. Also, there is some subtlety to the translation since most values need to be endian-translated, but literal strings do not. This is complexity we don't need.
2. Composability: The API is space-efficient: It does not copy the data in the underlying module. To get
   values from instructions, the client must have retained the base pointer to the module, and then index into it via the instruction offset and the operand offset. That's fine as far as it goes, but it isn't very composable. For example, suppose you want to chain multiple transforms together. The consumer at
   the end of the pipeline needs a base pointer somewhere to read values out of the instructions. But that means the last transform has to form a pointer relative to some array of words that was meaningful only at the beginning of the pipeline.

I think we can fix both problems in a reasonable way, as follows:
The spv_parsed_instruction_t should gain a "words" member of type "const uint32_t*". It points to native-endian sequence of SPIR-V words for the entire instruction. (So, for example, words[0] contains the combined word count and opcode.) As before, the contract is that the words array is only valid for as long as the callback is excecuting, then may be discarded or overwritten.

In the common case where the original module was in the machine's native endianness, then no copying is required: the words pointer is just computed from the original module's base pointer plus the instruction's offset. In the non-native endian case, there is some copying, but it's done by the parser and not the client. And since the lifetime of the data is restricted to the duration of the callback, we can reuse that space for the next instruction.

With the new design it will become easy to make pipelines of analysis or transform steps in a space efficient way. It should also be generally time efficient since the chain of callbacks should usually be working on the same chunk of instruction storage. It should be hot in the cache.

I assemble a simple program with an incorrect opcode at the end. spirv-as should report just that opcode as invalid, but instead it reports the whole program as the opcode name:

deki@deki-macbookpro>cat bug.spvasm 
OpCapability Shader
OpMemoryModel Logical GLSL450
OpXYZ
deki@deki-macbookpro>./spirv-as bug.spvasm 
error: 3: 1: Invalid Opcode name 'OpCapability Shader
OpMemoryModel Logical GLSL450
OpXYZ'

When spir.h changes, we should rebuild objects that depend on it.

This is the "proper" fix for #248
The temporary workaround committed was to stop checking capabilities on parameters that are IDs.

(No particular priority. I just wanted to record this rule while I had it in mind.)

Commit 61a6275 added a capability validation code that uses switch statements to get dependencies on capabilities.
All that data is already encoded in the AssemblyGrammar; see initialized tables in operand.cpp.
Duplicating the tables is a bad idea for maintenance reasons.

The parser is allowing an extended instruction to have too many parameters.
For example, the OpenCL pow instruction is only supposed to have two normal arguments, but the following is accepted by the assembler, and the result is happily disassembled by the binary-parser/disassembler.

          %1 = OpExtInstImport "OpenCL.std"
          %blam  = OpExtInst %a %1 pow %x %y %z
          OpNop

The %z argument should have triggered an error.

There's now an extension to GL to allow use of SPIR-V for shader programs:
https://www.opengl.org/registry/specs/ARB/gl_spirv.txt

We probably need to add entries to spv_target_env for any GL-based environments that will use SPIR-V. The gl_spirv extension provides validation rules.

The extension describes validation rules in seciton "A.spv.2" and allows for variations for OpenGL 4.0, 4.1, 4.2, 4.3, 4.5 due to progression of functionality in those specs.

Specifics
Windows 7, Nvidia GTX 660 Ti, Driver 368.39

Have Vulkan SDK 1.0.21 installed for validation layers and set environment variable VK_INSTANCE_LAYERS=VK_LAYER_LUNARG_standard_validation

To reproduce I run all of the dEQP-VK.glsl.functions.control_flow.* tests from the Vulkan Conformance Test Suite

Message:
Exception thrown: read access violation.
this was nullptr. (debugger confirms that variable "this" is NULL)

Location:
BasicBlock.h:

76 /// Returns true if the block is reachable in the CFG
77 bool reachable() const { return reachable_; }

Call Stack:

    VkLayer_core_validation.dll!libspirv::BasicBlock::reachable() Line 77   C++

    VkLayer_core_validation.dll!libspirv::StructuredControlFlowChecks(const libspirv::ValidationState_t & _, const libspirv::Function & function, const std::vector<std::pair<unsigned int,unsigned int>,std::allocator<std::pair<unsigned int,unsigned int> > > & back_edges) Line 349 C++

    VkLayer_core_validation.dll!libspirv::PerformCfgChecks(libspirv::ValidationState_t & _) Line 478    C++

    VkLayer_core_validation.dll!spvValidate(const spv_context_t * const context, spv_const_binary_t * const binary, spv_diagnostic_t * * pDiagnostic) Line 209  C++

    VkLayer_core_validation.dll!core_validation::CreateShaderModule(VkDevice_T * device, const VkShaderModuleCreateInfo * pCreateInfo, const VkAllocationCallbacks * pAllocator, VkShaderModule_T * * pShaderModule) Line 9069  C++

    VkLayer_object_tracker.dll!object_tracker::CreateShaderModule(VkDevice_T * device, const VkShaderModuleCreateInfo * pCreateInfo, const VkAllocationCallbacks * pAllocator, VkShaderModule_T * * pShaderModule) Line 3583    C++

    VkLayer_parameter_validation.dll!parameter_validation::CreateShaderModule(VkDevice_T * device, const VkShaderModuleCreateInfo * pCreateInfo, const VkAllocationCallbacks * pAllocator, VkShaderModule_T * * pShaderModule) Line 2621    C++

    VkLayer_threading.dll!threading::CreateShaderModule(VkDevice_T * device, const VkShaderModuleCreateInfo * pCreateInfo, const VkAllocationCallbacks * pAllocator, VkShaderModule_T * * pShaderModule) Line 1133  C++

    deqp-vk.exe!vk::DeviceDriver::createShaderModule(vk::VkDevice_s * device, const vk::VkShaderModuleCreateInfo * pCreateInfo, const vk::VkAllocationCallbacks * pAllocator, vk::Handle<14> * pShaderModule) Line 218  C++

    deqp-vk.exe!vk::createShaderModule(const vk::DeviceInterface & vk, vk::VkDevice_s * device, const vk::VkShaderModuleCreateInfo * pCreateInfo, const vk::VkAllocationCallbacks * pAllocator) Line 209    C++

    deqp-vk.exe!vk::createShaderModule(const vk::DeviceInterface & deviceInterface, vk::VkDevice_s * device, const vk::ProgramBinary & binary, unsigned int flags) Line 206 C++
    deqp-vk.exe!vkt::`anonymous namespace'::PipelineProgram::PipelineProgram(vkt::Context & context, const glu::sl::ShaderCaseSpecification & spec) Line 745    C++

    deqp-vk.exe!vkt::`anonymous namespace'::ShaderCaseInstance::ShaderCaseInstance(vkt::Context & context, const glu::sl::ShaderCaseSpecification & spec) Line 1410 C++

    deqp-vk.exe!vkt::`anonymous namespace'::ShaderCase::createInstance(vkt::Context & context) Line 1791    C++

    deqp-vk.exe!vkt::TestCaseExecutor::init(tcu::TestCase * testCase, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & casePath) Line 246   C++

    deqp-vk.exe!tcu::TestSessionExecutor::enterTestCase(tcu::TestCase * testCase, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & casePath) Line 181   C++

    deqp-vk.exe!tcu::TestSessionExecutor::iterate() Line 101    C++

    deqp-vk.exe!tcu::App::iterate() Line 172    C++

    deqp-vk.exe!main(int argc, const char * * argv) Line 55 C++
    [External Code] 

Bisected to spirv-tools commit: f61db0b
Validator structured flow checks: back-edge, constructs

Originally posted by @Tony-LunarG in google/shaderc#234:

An extra null pointer check is not needed in functions like the following.

• spvBinaryDestroy
• spvTextDestroy

Somewhat related to #45 . Fixes for that caused me to change the parser (to work around C++ library bugs on Mac). I also added tests to validate expected behaviour when parsing overflow numbers and non-numbers.

But those tests are now failing on Windows.
See AppVeyor results from github from testing github.com/google/shaderc
https://ci.appveyor.com/project/dneto0/shaderc/build/288/job/et8t59ppbyoe74n7
It appears that std::operator>>(float&) is broken on edge cases on that MSVC compiler and runtime.

[ RUN      ] FloatParse/ParseNormalFloatTest.Samples/1
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1033): error: Value of: parsed_value.value()
Expected: is equal to 0
  Actual: -0x1p-149
 literal: abc negate: true
[  FAILED  ] FloatParse/ParseNormalFloatTest.Samples/1, where GetParam() = 32-byte object <61-62 63-00 6F-46 33-32 2F-48 65-78 46-6C 6F-61 03-00 00-00 0F-00 00-00 01-00 65-73 00-00 00-00> (0 ms)

...

 RUN      ] FloatParse/ParseNormalFloatTest.Samples/14
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1033): error: Value of: parsed_value.value()
Expected: is equal to 3.40282e+38
  Actual: 0
 literal: 1e40 negate: false
[  FAILED  ] FloatParse/ParseNormalFloatTest.Samples/14, where GetParam() = 32-byte object <31-65 34-30 00-65 73-2F 31-33 65-78 46-6C 6F-61 04-00 00-00 0F-00 00-00 00-00 65-73 FF-FF 7F-7F> (0 ms)
[ RUN      ] FloatParse/ParseNormalFloatTest.Samples/15
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1033): error: Value of: parsed_value.value()
Expected: is equal to -3.40282e+38
  Actual: -0x1p-149
 literal: 1e40 negate: true 
[  FAILED  ] FloatParse/ParseNormalFloatTest.Samples/15, where GetParam() = 32-byte object <31-65 34-30 00-65 73-2F 31-33 65-78 46-6C 6F-61 04-00 00-00 0F-00 00-00 01-00 65-73 FF-FF 7F-FF> (1 ms)
[ RUN      ] FloatParse/ParseNormalFloatTest.Samples/16
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1033): error: Value of: parsed_value.value()
Expected: is equal to -3.40282e+38
  Actual: 0
 literal: -1e40 negate: false
[  FAILED  ] FloatParse/ParseNormalFloatTest.Samples/16, where GetParam() = 32-byte object <2D-31 65-34 30-00 73-2F 31-35 65-78 46-6C 6F-61 05-00 00-00 0F-00 00-00 00-00 65-73 FF-FF 7F-FF> (0 ms)

and

[ RUN      ] Float16Parse/ParseNormalFloat16Test.Samples/1
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1098): error: Value of: parsed_value.value()
Expected: is equal to 0x0p+0
  Actual: -0x0p+0
 literal: abc negate: true
[  FAILED  ] Float16Parse/ParseNormalFloat16Test.Samples/1, where GetParam() = 28-byte object <61-62 63-00 74-50 61-72 73-65 2F-50 61-72 73-65 03-00 00-00 0F-00 00-00 01-00 00-00> (0 ms)
[ RUN      ] Float16Parse/ParseNormalFloat16Test.Samples/2

and

 RUN      ] FloatOverflow/FloatProxyParseOverflowFloatTest.Sample/5
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1145): error: Value of: value.value().getAsFloat()
Expected: is equal to 3.40282e+38
  Actual: 0 (of type float)
[  FAILED  ] FloatOverflow/FloatProxyParseOverflowFloatTest.Sample/5, where GetParam() = 32-byte object <31-65 34-30 00-00 73-2F 31-32 2F-50 61-72 73-65 04-00 00-00 0F-00 00-00 00-00 00-00 FF-FF 7F-7F> (0 ms)
[ RUN      ] FloatOverflow/FloatProxyParseOverflowFloatTest.Sample/6
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1145): error: Value of: value.value().getAsFloat()
Expected: is equal to -3.40282e+38
  Actual: -1.4013e-45 (of type float)
[  FAILED  ] FloatOverflow/FloatProxyParseOverflowFloatTest.Sample/6, where GetParam() = 32-byte object <2D-31 65-34 30-00 73-2F 31-32 2F-50 61-72 73-65 05-00 00-00 0F-00 00-00 00-00 00-00 FF-FF 7F-FF> (0 ms)
[ RUN      ] FloatOverflow/FloatProxyParseOverflowFloatTest.Sample/7
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1145): error: Value of: value.value().getAsFloat()
Expected: is equal to 3.40282e+38
  Actual: 0 (of type float)
[  FAILED  ] FloatOverflow/FloatProxyParseOverflowFloatTest.Sample/7, where GetParam() = 32-byte object <31-65 34-30 30-00 73-2F 31-32 2F-50 61-72 73-65 05-00 00-00 0F-00 00-00 00-00 00-00 FF-FF 7F-7F> (0 ms)
[ RUN      ] FloatOverflow/FloatProxyParseOverflowFloatTest.Sample/8
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1145): error: Value of: value.value().getAsFloat()
Expected: is equal to -3.40282e+38
  Actual: -1.4013e-45 (of type float)
[  FAILED  ] FloatOverflow/FloatProxyParseOverflowFloatTest.Sample/8, where GetParam() = 32-byte object <2D-31 65-34 30-30 00-2F 31-32 2F-50 61-72 73-65 06-00 00-00 0F-00 00-00 00-00 00-00 FF-FF 7F-FF> (0 ms)
[----------] 9 tests from FloatOverflow/FloatProxyParseOverflowFloatTest (1 ms total)

and

 RUN      ] DoubleOverflow/FloatProxyParseOverflowDoubleTest.Sample/7
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1170): error: Value of: value.value().getAsFloat()
Expected: is equal to 1.79769e+308
  Actual: 1.53657e-304 (of type double)
[  FAILED  ] DoubleOverflow/FloatProxyParseOverflowDoubleTest.Sample/7, where GetParam() = 40-byte object <31-65 34-30 30-00 1D-01 60-C7 1D-01 01-00 00-00 05-00 00-00 0F-00 00-00 00-00 00-00 00-00 00-00 FF-FF FF-FF FF-FF EF-7F> (0 ms)
[ RUN      ] DoubleOverflow/FloatProxyParseOverflowDoubleTest.Sample/8
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1170): error: Value of: value.value().getAsFloat()
Expected: is equal to -1.79769e+308
  Actual: -1.53657e-304 (of type double)
[  FAILED  ] DoubleOverflow/FloatProxyParseOverflowDoubleTest.Sample/8, where GetParam() = 40-byte object <2D-31 65-34 30-30 00-00 00-00 00-00 00-00 00-00 06-00 00-00 0F-00 00-00 00-00 00-00 00-00 00-00 FF-FF FF-FF FF-FF EF-FF> (1 ms)
[----------] 9 tests from DoubleOverflow/FloatProxyParseOverflowDoubleTest (1 ms total)

and

 RUN      ] Float16Overflow/FloatProxyParseOverflowFloat16Test.Sample/5
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1194): error: Expected: (GetParam().expect_success) != (input.fail()), actual: true vs true
 literal: 1e40
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1196): error: Value of: value.value().data()
Expected: is equal to 31744
  Actual: 0 (of type unsigned short)
 literal: 1e40
[  FAILED  ] Float16Overflow/FloatProxyParseOverflowFloat16Test.Sample/5, where GetParam() = 28-byte object <31-65 34-30 00-00 2F-32 76-65 72-66 6C-6F 77-2F 04-00 00-00 0F-00 00-00 01-00 00-7C> (0 ms)
[ RUN      ] Float16Overflow/FloatProxyParseOverflowFloat16Test.Sample/6
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1194): error: Expected: (GetParam().expect_success) != (input.fail()), actual: true vs true
 literal: 1e400
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1196): error: Value of: value.value().data()
Expected: is equal to 31744
  Actual: 0 (of type unsigned short)
 literal: 1e400
[  FAILED  ] Float16Overflow/FloatProxyParseOverflowFloat16Test.Sample/6, where GetParam() = 28-byte object <31-65 34-30 30-00 2F-32 76-65 72-66 6C-6F 77-2F 05-00 00-00 0F-00 00-00 01-00 00-7C> (1 ms)
[ RUN      ] Float16Overflow/FloatProxyParseOverflowFloat16Test.Sample/7
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1194): error: Expected: (GetParam().expect_success) != (input.fail()), actual: true vs true
 literal: -1e40
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1196): error: Value of: value.value().data()
Expected: is equal to 64512
  Actual: 32768 (of type unsigned short)
 literal: -1e40
[  FAILED  ] Float16Overflow/FloatProxyParseOverflowFloat16Test.Sample/7, where GetParam() = 28-byte object <2D-31 65-34 30-00 2F-32 76-65 72-66 6C-6F 77-2F 05-00 00-00 0F-00 00-00 01-00 00-FC> (0 ms)
[ RUN      ] Float16Overflow/FloatProxyParseOverflowFloat16Test.Sample/8
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1194): error: Expected: (GetParam().expect_success) != (input.fail()), actual: true vs true
 literal: -1e400
C:\projects\shaderc\third_party\spirv-tools\test\HexFloat.cpp(1196): error: Value of: value.value().data()
Expected: is equal to 64512
  Actual: 32768 (of type unsigned short)
 literal: -1e400
[  FAILED  ] Float16Overflow/FloatProxyParseOverflowFloat16Test.Sample/8, where GetParam() = 28-byte object <2D-31 65-34 30-30 00-32 76-65 72-66 6C-6F 77-2F 06-00 00-00 0F-00 00-00 01-00 00-FC> (0 ms)
[----------] 9 tests from Float16Overflow/FloatProxyParseOverflowFloat16Test (1 ms total)

Also, there's an assert failure if the last instruction has a word count that extends beyond the input.

I have fixes for both queued up, buliding on #28

I'm trying to build SPIRV-Tools under gentoo linux (with python-3.5) and I run into the following issues:

[ 3%] Update build-version.inc in the Spirv-tools build directory (if necessary).
/usr/bin/python3.5 /home/benjamin/Software/SPIRV-Tools/utils/update_build_version.py /home/benjamin/Software/SPIRV-Tools
File "/home/benjamin/Software/SPIRV-Tools/utils/update_build_version.py", line 67
print 'usage: {0} <spirv-tools_dir>'.format(sys.argv[0])
^
SyntaxError: invalid syntax

I commented out line 67 as it is likely not used during build but then I run into this issue:

[ 3%] Update build-version.inc in the Spirv-tools build directory (if necessary).
/usr/bin/python3.5 /home/benjamin/Software/SPIRV-Tools/utils/update_build_version.py /home/benjamin/Software/SPIRV-Tools
Traceback (most recent call last):
File "/home/benjamin/Software/SPIRV-Tools/utils/update_build_version.py", line 76, in
main()
File "/home/benjamin/Software/SPIRV-Tools/utils/update_build_version.py", line 70, in main
new_content = ('"spirv-tools ' + describe(sys.argv[1]).replace('"', '"') + '\n"\n')
TypeError: a bytes-like object is required, not 'str'

As my python experience is extremely limited, I thought I should just file this.

The following should assemble, but should not disassemble.

   OpDecorate %1 LinkageAttributes "abc" !1 !0

The !1 is 1, which is interpreted as linkage attribute "Import". The instruction ends up having 6 words, the last being value 0.
However, the binary parser (and disassembler) should reject this input because that last !0 word is extraneous.

The root cause is that the grammar in opcode.inc has a last operand class of OperandVariableLiterals, so that expands to zero or more literals. Instead that should be dropped, and we should allow processing of the decoration enum word to add just the right number and type of operands to the operand pattern.

The Image arg to OpImageFetch must be of type OpTypeImage. Currently OpSampledImage is not causing an error. glslang was previously generating OpSampledImage value but is now adding an OpImage operator.

Triggered by this input:

           OpCapability Shader
           OpMemoryModel Logical GLSL450
%voidt   = OpTypeVoid
%funct   = OpTypeFunction %voidt

%blah    = OpFunction %voidt None %funct
%entry   = OpLabel
           OpBranch %loop
%loop    = OpLabel
           OpLoopMerge %exit %loop None
           OpBranch %loop
%exit    = OpLabel
           OpReturn
           OpFunctionEnd

It's stuck in libspirv::CalculateDominators

That's a useful behaviour, to simplify client code.
The current implementation is already safe in this way.
Need to document it and write tests for it.

Macros like CHECK can easily become functions.

Building on Ubuntu and getting this message.

Need to fix the code or remove pedantic?

Thanks

There is a lot of useful functionality, especially in opcode.h which is currently not exposed via the libspirv.h header.

I'm mostly looking for an easy way to get instruction metadata (e.g. names of instructions and types of operands). I'm generating this myself from the HTML spec at https://www.khronos.org/registry/spir-v/specs/1.0/SPIRV.html right now, but this is cumbersome to update since the HTML formatting tends to change slightly between versions.

One thing that I found particularly useful while working on https://github.com/bonus2113/otherside/ was having a type for each instruction (see https://github.com/bonus2113/otherside/blob/master/src/shared/lookups_gen.h for an example) which allowed me to easily access operands by name, but that might be out of scope for this project.

It's a very common operation. It should use a lower complexity (by op count) algorithm.

Secondarily, the FindUsesWithDefs is algorithmically inefficient, and perhaps could be fixed at the same time.

There is a set of non-trivial constraints in SPIR-V & Vulkan specs regarding decorating variables with:

• location & binding qualifiers
• interpolation and patch qualifiers
• offset and alignment

Especially nested interface blocks / structs / arrays need to follow rules defined in "14.1. Shader Input and Output Interfaces" Vulkan spec chapter to get SPIR-V shaders properly "linked" during pipeline creation.

As there are apps (including glslang - KhronosGroup/glslang#269) misusing the decorations it would be good to have these checked in SPIR-V validator.

Including libspirv.h in a C project causes an error at this line :
https://github.com/KhronosGroup/SPIRV-Tools/blob/master/include/spirv-tools/libspirv.h#L344
because spv_context_t isn't a known type, but struct spv_context_t is.

Since libspirv is intended as a C API, this should be fixed by adding
typedef struct spv_context_t spv_context_t; just here :
https://github.com/KhronosGroup/SPIRV-Tools/blob/master/include/spirv-tools/libspirv.h#L334

Let's discuss whether and how we can get rid of these options. I suspect that the vast majority of users doesn't need sophisticated ways to turn off certain validations. Furthermore, the options are intertwined in unexpected ways -- for instance, multiple validations need _.current_layout_section_, but this is only computed when SPV_VALIDATE_LAYOUT_BIT is set.

Unit tests also shouldn't need fine-grained controls of this kind -- those that do aren't really unit tests. Moreover, it's fishy design to add so much API just for testing purposes.

My opinion is that we should eliminate these controls altogether, perform full validation always, and fix unit tests to have minimally invalid assembly or invoke only specific validations directly.

@umar456 @dneto0 @AWoloszyn ^^

The following tests are failing for parseNumber using clang++ Apple LLVM version 6.0 (clang-600.0.57) (based on LLVM 3.5svn).:

[  FAILED  ] AssemblyContextParseNarrowUnsignedIntegers.Sample
[  FAILED  ] AssemblyContextParseWideUnsignedIntegers.Sample
[  FAILED  ] AssemblyContextParseFloat.Sample
[  FAILED  ] AssemblyContextParseDouble.Sample

This is because clang does not fail for large numbers(i.e. 1e400) when converting text to float using istringstream.

#include <iostream>
#include <iomanip>
#include <sstream>
#include <limits>
using namespace std;

int main() {
  istringstream text_stream("1e400");
  double val = 0;

  text_stream >> std::setbase(0);
  text_stream >> val;

  cout << text_stream.fail() << endl;
  cout << "val: " << val << endl;
}

Output:

clang++ -std=c++11 foo.cpp -o clang_foo && echo clang && ./clang_foo && g++-4.8 -std=c++11 foo.cpp -o gcc_foo && echo gcc && ./gcc_foo
clang
0
val: inf
gcc
1
val: 1.79769e+308

There is open debate about when a ClipDistance or CullDistance capability is required.
For example, see KhronosGroup/glslang#334
Until the dust settles, it might be prudent to disable those capability checks.

I got following error for the attached SPIR-V file:

error: 350: OpReturnValue value's type <id> '10' is a pointer, but a pointer can only be an operand to OpLoad, OpStore, OpAccessChain, or OpInBoundsAccessChain.

The list of the instructions accepting pointer operands is too short. There are a lot more instructions (especially with Kernel capability) that have pointer operands.

When writing structs to an SSBO this validation error triggers:

error: 196: OpStore Pointer <id> '10's type does not match Object <id> '11's type.

SPIR-V:

; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 1
; Bound: 25
; Schema: 0
               OpCapability Shader
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint GLCompute %4 "main"
               OpExecutionMode %4 LocalSize 1 1 1
               OpSource ESSL 310
               OpName %4 "main"
               OpName %8 "MyStruct"
               OpMemberName %8 0 "foo"
               OpName %10 "bar"
               OpName %11 "MyStruct"
               OpMemberName %11 0 "foo"
               OpName %12 "SSBO0"
               OpMemberName %12 0 "foo_std430"
               OpName %14 ""
               OpName %20 "SSBO1"
               OpMemberName %20 0 "foo_std430_out"
               OpName %22 ""
               OpMemberDecorate %11 0 Offset 0
               OpMemberDecorate %12 0 Offset 0
               OpDecorate %12 BufferBlock
               OpDecorate %14 DescriptorSet 0
               OpDecorate %14 Binding 0
               OpMemberDecorate %20 0 Offset 0
               OpDecorate %20 BufferBlock
               OpDecorate %22 DescriptorSet 0
               OpDecorate %22 Binding 1
          %2 = OpTypeVoid
          %3 = OpTypeFunction %2
          %6 = OpTypeFloat 32
          %7 = OpTypeVector %6 4
          %8 = OpTypeStruct %7
          %9 = OpTypePointer Function %8
         %11 = OpTypeStruct %7
         %12 = OpTypeStruct %11
         %13 = OpTypePointer Uniform %12
         %14 = OpVariable %13 Uniform
         %15 = OpTypeInt 32 1
         %16 = OpConstant %15 0
         %17 = OpTypePointer Uniform %11
         %20 = OpTypeStruct %11
         %21 = OpTypePointer Uniform %20
         %22 = OpVariable %21 Uniform
          %4 = OpFunction %2 None %3
          %5 = OpLabel
         %10 = OpVariable %9 Function
         %18 = OpAccessChain %17 %14 %16
         %19 = OpLoad %11 %18
               OpStore %10 %19
         %23 = OpLoad %8 %10
         %24 = OpAccessChain %17 %22 %16
               OpStore %24 %23
               OpReturn
               OpFunctionEnd

Generated from this glslang output:

#version 310 es
layout(local_size_x = 1) in;

struct MyStruct
{
    vec4 foo;
};

layout(binding = 0, std430) buffer SSBO0
{
    MyStruct foo_std430;
};

layout(binding = 1, std430) buffer SSBO1
{
    MyStruct foo_std430_out;
};

void main()
{
    MyStruct bar = foo_std430;
    foo_std430_out = bar;
}

The SPIR-V Tools API is a C API to provide ABI stability and a lowest-common-denominator target for multiple languages. However, it's begging for a more convenient C++ API.

Example code:

               OpCapability Shader
               OpCapability Float64
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Vertex %main "main"
               OpSource GLSL 450
               OpSourceExtension "GL_GOOGLE_cpp_style_line_directive"
               OpSourceExtension "GL_GOOGLE_include_directive"
               OpName %main "main"
               OpDecorate %7 SpecId 200
               OpDecorate %9 SpecId 201
               OpDecorate %11 SpecId 202
               OpDecorate %13 SpecId 203
       %void = OpTypeVoid
          %3 = OpTypeFunction %void
        %int = OpTypeInt 32 1
          %7 = OpConstant %int 3
      %float = OpTypeFloat 32
          %9 = OpConstant %float 3.14
     %double = OpTypeFloat 64
         %11 = OpConstant %double 3.14159265358979
       %bool = OpTypeBool
         %13 = OpConstantTrue %bool
       %main = OpFunction %void None %3
          %5 = OpLabel
               OpReturn
               OpFunctionEnd

This code is invalid as SpecId should not be used to decorate a normal constant. But spirv-val does not alert on this case.

The assembler puts the spec revision number it's based on into the bottom byte of the version header word. But SPIR-V 1.0 says those bottom bits must be 0.

The disassembler is printing that bottom byte, but since it's always zero, we probably shouldn't print it.

See https://www.khronos.org/bugzilla/show_bug.cgi?id=1469

We'll wait for the fix in the next revision of SPIR-V 1.0, then implement it here in SPIR-V Tools.

The SPIR-V example on the SPIR-V specification (under 'Corresponding SPIR-V:') is rejected with saying error: 123: 16: Invalid Opcode name '; Magic: 0x07230203 (SPIR-V).

When I remove the comment lines starting with a semicolon, then I get the following output:
error: 119: 16: Invalid Opcode name '
the source file has as very last line OpFunctionEnd

If I add a single line break to the file, it works. I think that line break problem is described by #126.

Running spirv-as on the attached file yields:

error: 21: 1: Invalid Opcode name 'OpCapability Shader
OpMemoryModel Logical GLSL450

%i32 = OpTypeInt 32 1
%f32 = OpTypeFloat 32
%v4i32 = OpTypeVector %i32 4
%timg = OpTypeImage %i32 2D 0 0 0 0 Unknown
%pimg = OpTypePointer Input %timg
%tfun = OpTypeFunction %i32

%vimg = OpVariable %pimg Input
%coord = OpConstant %i32 0
%bias = OpConstant %f32 0.

%main = OpFunction %i32 None %tfun
%lbl = OpLabel
%img = OpLoad %timg %vimg
%r1 = OpImageFetch %v4i32 %img %coord Offset|Bias %bias %bias
;%r2 = OpImageSampleExplicitLod %v4i32 %sample %coord Bias 0.2
OpReturnValue %coord
OpFunctionEndѷ'

IOW, it thinks the whole program is an opcode name. 🙀

shader.txt

These should be in the libspirv namespace.

There is a tiny inconvenience in OpenCL.std.h included by libspirv.h. That OpenCL header file is a C++ header file and so a client including libspirv.h cannot compile as C application, but must use a C++ compiler. (msvc complains when compiling a .c file including spirv-tools\libspirv.h)
Simply guarding the "namespace" directive with an #ifdef __cplusplus , or removing that namespace declaration, helped.

The name of TextToBinaryTest[Base] is outdated; we actually have roundtrip test methods inside it. I'd like to change it to RoundTripTest[Base]. And also, rename the methods inside it to something like {Encode|Decode}...{Succeeded|Failed}....

Opinions?

This issue tracks validator changes for changes expected from resolution to KhronosGroup/SPIRV-Headers#16

Related to
#270

Currently, the validator disallows an unreachable continue construct for a loop. I expect we should relax this rule.

My understanding is that SPIR-V v1.0 specification doesn't force control flow to be structured. OpenCL C support things like 'goto' and OpenCL C -> SPIR-V compiler from here produces SPIR-V files with unstructured control flow. These SPIR-V files do not pass structured control flow checks.
Can we make them optional for SPIR-V files produced from OpenCL C?

I attached CL source and SPIR-V file demonstrating the issue.

sample_test.zip

I built a small DLL using the SPIRV-Tools.lib library to allow another language to call into SPIRV-Tools using a foreign-function interface of that language to call into C libraries.
The function in the DLL accepted a single memory pointer to the assembly text, which was loaded by the client program before calling into the DLL and eventually into spvTextToBinary.
This assembly text was read from a file as is, and not via fread, which in "r" text mode apparently converts CR+LF to LF under msvc 14.
The function spvTextToBinary() seems to be unable to handle CR+LF linebreaks in the source assembly text.
The spirv-as assembly tool works flawlessly, though, because fread (in "r" text mode) converts CR+LF to LF (\n).
But it does not work when using other mechanism that read the file as is without any conversion of line-breaks.

So the current workaround is to force the client of spvTextToBinary() to convert CR+LF to LF before handing the assembly text to spvTextToBinary.

Error message I am getting for the example in the SPIR-V specification:

error: 6: 1: Expected <opcode> or <result-id> at the beginning of an instruction, found '
'.

libspirv.h currently includes the spirv headers from "headers/...". This is inconvenient in a public header like this as it makes all code including it need to have an additional include path set up to the spirv-tools/external/include/ path.

An easy fix would be to remove the "headers/" from the include paths such that projects need only set up an include path for spirv (which they most likely already have, or can do in a variety of ways).

If SPIRV-Tools may need non-standard modifications to the headers and cannot use the existing ones someone may have then changing the includes to be unambiguous and relative to libspirv.h would make more sense ("../../external/includes/headers/...").

For example, see https://travis-ci.org/KhronosGroup/SPIRV-Tools/builds/102705182

Until this is fixed, we have to ignore failures from the Travis-CI

This affects a Vulkan CTS test.

The essence of the case is:

           OpCapability Shader
           OpMemoryModel Logical GLSL450

           OpName %copy "copy"
           OpName %false "false"
           OpName %if_true "if_true"
           OpName %exit "exit"

%voidt   = OpTypeVoid
%funct   = OpTypeFunction %voidt
%boolt   = OpTypeBool
%false   = OpConstantFalse %boolt

%main    = OpFunction %voidt None %funct
%entry   = OpLabel
           OpSelectionMerge %exit None
           OpBranchConditional %false %if_true %exit

%if_true = OpLabel
%copy    = OpCopyObject %boolt %false
           OpBranch %exit

%exit    = OpLabel
%v       = OpPhi %boolt %false %entry %copy %if_true
           OpReturn
           OpFunctionEnd

The error reported is: error: 21: ID 1[copy] defined in block 3[if_true] does not dominate its use in block 4[exit]

Use Case:
Unit test in the validation rely on the error codes validate the result of the spvValidate function. Ideally the tests should check both the error code and the diagnostic message to verify that the test case has accurately passed. This is currently not possible because the named ID representation is lost during the Text to Binary conversion.

The spvTextToBinary function should return a map which contains a mapping from ID to ID names used in the textual representation of the SPIR-V.

In particular during validation of atomic instructions memory semantics operands are treated as literals, but according to the spec they must be id.

In the attached example, it happened that OpAtomicIAdd has memory semantics operand = %64. Literally it corresponds to 0x40 (UniformMemory, which require shader capability). But actually %64 is an id of an integer constant = 0x10(SequentiallyConsistent, no capability required).

%64 = OpConstant %2 16
...
%63 = OpAtomicIAdd %2 %50 %35 %64 %35

This causes false positive errors to be reported by spirv-val tool:

error: 129: Operand 5 of AtomicIAdd requires one of these capabilities: Shader

atomic_exchange.global_atomic_double_declared_in_program.used_in_generic_function.order_relaxed.spv20_32.zip

atomic_exchange.global_atomic_double_declared_in_program.used_in_generic_function.order_relaxed.spv20_32.txt

I got following error message for the attached SPIR-V file:

error: 25: A function must begin with a label

The disassembled module part looks like this:

     %12 = OpFunction %6 None %11
     %13 = OpFunctionParameter %8
     %14 = OpFunctionParameter %10
     %15 = OpLabel

According to specification function definition layout is:

• Function definition, using OpFunction.
• Function parameter declarations, using OpFunctionParameter.
• Block
• Block
• …
• Function end, using OpFunctionEnd.

Block must start with OpLabel instruction.

It looks like SPIR-V validation tools doesn't expect OpFunctionParameter to follow OpFunction instruction.

Function declaration layout check has the same issue.

希望增加visual studio 工程项目。
把一个需要到的文件都放到一个项目里，这样的话也方便了新手 嗯

The binary parser uses "index" to indicate word offset into the binary.
The validator pull request uses "index" as the instruction count.
#27

It's not 100% clear which is better, and the definition of spv_position_t is silent on this point.

Need to decide what is the right interpretation, and stick to it.

Currently, GetModuleOrder() returns a reference to a static vector object. This is a violation of the style guide because other static objects may accidentally dereference this in their destructors, without any guarantee that the GetModuleOrder vector isn't already destroyed. Instead of creating a static vector, the method should either use a C array or a static pointer to a dynamically allocated vector.

References:
https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables
https://isocpp.org/wiki/faq/ctors#construct-on-first-use-v2

The code currently assumes that strings can be directly read in and written to disk without being swapped, but this isn't true. The SPIR-V spec, in the definition of "literal string" (section 2.2.1), says that "the UTF-8 octets (8-bit bytes) are packed four per word, following the little-endian convention (i.e., the first octet is in the lowest-order 8 bits of the word)." That is, literal strings are defined in terms of words, just like everything else in SPIR-V. While the way that bytes on-disk get transformed to words isn't defined by the spec, it wouldn't make any sense to define it "just so" so that the way SPIR-V Tools is currently parsing the SPIR-V would be correct, since it would make the definition in section 2.2.1 confusing and meaningless, and I can say that that wasn't the intent when the language in section 2.2.1 was added. That is, it's expected that when reading from byte-oriented formats, if the magic word is flipped, then the entire module is flipped. Practically, this also makes loading SPIR-V from disk in games much easier, since they don't need to know about what's a string vs. what isn't -- just check the magic word and flip the bytes if necessary.