Is there a way to make glslang generate some basic spirv using intermediate
object ids which would be used in an opphi instruction (which can use only intermediate object id).
It seems the variable/opstore/opload work around is always used.

According to latest SPIRV spec, the first source operand "Pointer" should be a pointer variable. Because OpAtomicXXX will be applied to shader image atomic operations, this pointer could be a pointer to a variable or be an image texel pointer.

GLSL:

version 450

buffer Buffer
{
uint u1;
} buf;

void main()
{
atomicAdd(buf.u1, 1);
}

Faulted SPIRV:
16: 7(int) Constant 1
...
14: 13(ptr) AccessChain 10(buf) 12
15: 7(int) Load 14
17: 7(int) AtomicIAdd 15 Device None 16

Correct SPIRV
15: 7(int) Constant 1
...
14: 13(ptr) AccessChain 10(buf) 12
16: 7(int) AtomicIAdd 14 Device None 15

The load operation is unnecessary.

The GLSL and ESSL specifications say the following about the order of evaluation of the logical operators in 5.9 Expressions

The logical binary operators and (&&), or (||), and exclusive or (^^) operate only on two
Boolean expressions and result in a Boolean expression. And (&&) will only evaluate the right
hand operand if the left hand operand evaluated to true. Or (||) will only evaluate the right hand
operand if the left hand operand evaluated to false. Exclusive or (^^) will always evaluate both
operands.

This behavior doesn't seem to be translated to SPIR-V. The code always evaluates both sides and combines them with OpLogicalXX. Wouldn't the correct behavior be a conditional branch to evaluate the second operand depending on the first?

In Geometry shaders, you are required to specify the input and output primitive type in the GLSL code. In SPIR-V this is represented by SpvExecutionModeInput* and SpvExecutionModeOutput* execution modes. However, glslang doesn't provide this information in the shader.

bitfieldInsert/Extract/Reverse et. al. are currently unimplemented in SPIR-V output.

#version 310 es

layout(location = 0) out int FragColor;
flat in int v0;
flat in int v1;

void main()
{
    FragColor = bitfieldInsert(v0, v1, 10, 5);
}

test.frag
Warning, version 310 is not yet complete; most version-specific features are present, but some are missing.


Linked fragment stage:


Missing functionality: integer aggregate

version 450

layout(location = 0) in vec4 in_position;

void main()
{
float y;

    if(in_position[0]>50.0)
            y=1.0;
    else
            y=2.0;

    gl_Position[0] = y;

}

It seems to generate invalid SSA because it is assigning 2 times a variable, probably because it misses a phi SSA function.
Or do I miss something?

This shader curiously results in a segmentation fault.

#version 120
vec3 m(vec2);
vec3 o() {
  m(ivec2(O));
}

Funnily enough, it also happens when the #version 120 has no space between the directive and its argument(that is, #version120), not sure what that's about.

The input "#line 0%\n" when interpereted as a fragment shader causes glslangValidator to crash with SIGFPE in Pp.cpp (op_mod).

We plan to implement the #include and #line directive support via GLSL extensions, so the default glslang behaviour remains as today. We believe it will benefit the community to put the extension implementation into glslang.

Specifically, we plan to introduce two extensions: GL_GOOGLE_cpp_style_line_directive and GL_GOOGLE_include_directive. The first one allows #line to accept a constant string (for a filename) as its second parameter and substitute __FILE__ accordingly. The second one enables #include support.

There will be a couple of commits for this issue, falling into the following three categories:

1. preliminary work to make the preprocessor handle line numbers correctly
2. add support for GL_GOOGLE_cpp_style_line_direcitve
3. add support for GL_GOOGLE_include_directive

I am working on a Spir-V Decompiler and am looking at some binaries produced by glslang. It incorrectly generates opcode 54 (OpFunction) with only a 4 byte word count, followed by multiple other OpFunction opcodes. This is incorrect, because it should be immediately followed by 55, OpFunctionParameter, for each argument, and then followed by a 56 for OpFuncitonEnd. There is no generated OpFunctionParameter or OpFuncitonEnd in the created binary. Both source and binary are attached. Compiled using version 2.2.687

"#if\377" triggers an issue in glslangValidator causing an infinite hot loop which also allocates memory until it crashes.

Seems like a bug since it's inconsistent with geometry shader behavior for example.

#version 450

layout(fractional_even_spacing, quads, ccw, point_mode) in;
patch in vec4 vFoo;

void main()
{
    gl_Position = vec4(gl_in[0].gl_Position.xy + vFoo.xy + gl_TessCoord.xy, 0.0, 1.0);
}

                          Source GLSL 450
                          Capability Tessellation
           1:             ExtInstImport  "GLSL.std.450"
                          MemoryModel Logical GLSL450
                          EntryPoint TessellationEvaluation 4  "main"
                          ExecutionMode 4 InputQuads
                          Name 4  "main"
                          Name 11  "gl_PerVertex"
                          MemberName 11(gl_PerVertex) 0  "gl_Position"
                          MemberName 11(gl_PerVertex) 1  "gl_PointSize"
                          MemberName 11(gl_PerVertex) 2  "gl_ClipDistance"
                          MemberName 11(gl_PerVertex) 3  "gl_CullDistance"
                          Name 13  ""
                          Name 16  "gl_PerVertex"
                          MemberName 16(gl_PerVertex) 0  "gl_Position"
                          MemberName 16(gl_PerVertex) 1  "gl_PointSize"
                          MemberName 16(gl_PerVertex) 2  "gl_ClipDistance"
                          MemberName 16(gl_PerVertex) 3  "gl_CullDistance"
                          Name 20  "gl_in"
                          Name 26  "vFoo"
                          Name 32  "gl_TessCoord"
                          MemberDecorate 11(gl_PerVertex) 0 BuiltIn Position
                          MemberDecorate 11(gl_PerVertex) 1 BuiltIn PointSize
                          MemberDecorate 11(gl_PerVertex) 2 BuiltIn ClipDistance
                          MemberDecorate 11(gl_PerVertex) 3 BuiltIn CullDistance
                          Decorate 11(gl_PerVertex) Block *Associated with gl_out, marked builtin*
                          Decorate 16(gl_PerVertex) Block *The input is not*

The SPIR-V generator currently uses unsigned int for all its internal representation. However there is no guarantee for this type to be 32 bits (one SPIR-V word) wide.

There are two solutions to this:

• Change the official Khronos headers to generate uint32_t for the Id typedef and underlying types for enums.
• Change the glslang code to only use uint32_t internally.

I would prefer the first option (together with other improvements to the C++ headers) but that requires changing the Khronos code generator and I have no idea whether that is public and if so where to find it/contribute to it (the Khronos bugtracker only has a section for the specification, not the interface files).

In any case, the glslang code as it is right now may produce faulty SPIR-V binaries or fail to load valid SPIR-V binaries if the condition sizeof(unsigned int) * CHAR_BIT == 32 does not hold on the host platform. As a plus, if the platform does not have an exact 32 bit wide integer the code would fail to compile as the exactly sized typedefs are optional.

There may be similar issues with float and double if they don't follow the IEEE 754 standard or have unexpected sizes.

Git is very fast and efficient for tracking line-oriented text files, however it is fairly weak at working with binaries. The common case is that git will rewrite the entire binary file, and both will need to be downloaded with the repository, making Git clones(and some other operations) slower than they need to be. It will get far slower over time if binaries are updated in the repository more and more times.

Github has a mechanism for releasing binaries. It's pretty convenient and makes it simple for people to pick up older releases if the need may arise. It also allows you to do pre-releases, which people are not encouraged to use in a production setting, but can try out nonetheless.

Another benefit of this, is that in the future the build process can be automated and releases can be updated every time you push a release tag.

When trying to compile a shader which contains struct comparison the following error is reported: Missing functionality: Composite comparison of non-vectors.

Test case:

#version 140
in highp vec4 a_position;

struct S { int a; int b; };

void main (void)
{
    S a, b;
    if(a==b) {}

    gl_Postion = a_position;
}

Compiled with the glslangValidator test.vert -V command.

#extension 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

Smells like it's jumping into some undefined memory, which seems to me like a liability to me.

When running the code through the clang static analyzer (for instance in Xcode -> Product -> Analyzer), there's a small number of warnings (mostly uncritical like 'Values stored to X is never read'), but there are a couple of more critical warning in glslangValidator/StandAlone.cpp where memory is not freed, and a function is potentially called with an unitialized value (very likely a false positive because the code should return, but easy to fix).

Here's a list of the critical warnings (everything that is not a 'is never read' warning):

Potential leak of memory pointed to by 'config' in StandAlone.cpp/ProcessConfigFile():

If there's no config file given, a default 'config' is used which is allocated here: https://github.com/KhronosGroup/glslang/blob/master/StandAlone/StandAlone.cpp#L239, the static analyzer thinks that is is never freed.

Apart from that warning, there are several allocations happening in ReadFileData(), which don't seem to have a corresponding free() call (for instance here: https://github.com/KhronosGroup/glslang/blob/master/StandAlone/StandAlone.cpp#L965.

Also in FreeFileData() it looks like only the 'node pointers' are freed, not the array that contains the pointers allocated here: https://github.com/KhronosGroup/glslang/blob/master/StandAlone/StandAlone.cpp#L953

Potential leak of memory pointed to by 'program':

The object created here: https://github.com/KhronosGroup/glslang/blob/master/StandAlone/StandAlone.cpp#L619

...is not deleted when this if is taken: https://github.com/KhronosGroup/glslang/blob/master/StandAlone/StandAlone.cpp#L627

Potential leak of memory pointed to by 'return_data':

The memory allocated here: https://github.com/KhronosGroup/glslang/blob/master/StandAlone/StandAlone.cpp#L953

Is not freed when this 'if' is taken: https://github.com/KhronosGroup/glslang/blob/master/StandAlone/StandAlone.cpp#L956

Function call argument is an unitialized value:

This is very likely a false positive, but easy to fix: clang analyzer says that the 'FILE* in' here: https://github.com/KhronosGroup/glslang/blob/master/StandAlone/StandAlone.cpp#L948

...can remain unitialized if the call to fopen_s returns with an error when fgetc() is called here: https://github.com/KhronosGroup/glslang/blob/master/StandAlone/StandAlone.cpp#L960

This is the list of 'Value stored in X is never read', some of these may point to actual bugs, not sure:

Value stored to 'ch' is never read:

https://github.com/KhronosGroup/glslang/blob/master/glslang/MachineIndependent/preprocessor/PpContext.h#L411

Value stored to 'fragOutHasLocation' is never read:

https://github.com/KhronosGroup/glslang/blob/master/glslang/MachineIndependent/linkValidate.cpp#L557

Value stored to 'token' is never read:

https://github.com/KhronosGroup/glslang/blob/master/glslang/MachineIndependent/preprocessor/Pp.cpp#L831

Value stored to 'isVersion' is never read:

https://github.com/KhronosGroup/glslang/blob/master/glslang/MachineIndependent/preprocessor/Pp.cpp#L867

Value stored to 'exp' is never read:

https://github.com/KhronosGroup/glslang/blob/master/glslang/MachineIndependent/preprocessor/PpScanner.cpp#L182

Value stored to 'word' is never read:

https://github.com/KhronosGroup/glslang/blob/master/SPIRV/SPVRemapper.cpp#L457
https://github.com/KhronosGroup/glslang/blob/master/SPIRV/SPVRemapper.cpp#L469

Value stored to 'nextInst' during its initialization is never read:

https://github.com/KhronosGroup/glslang/blob/master/SPIRV/SPVRemapper.cpp#L469

And that is all :) I can provide a pull request for most of these if you want (the only part that's a bit more complex is the allocation/free stuff around ReadFileData / FreeFileData).

Hello.

Given m.vert:

#version 330 core

vec4 f();

void
main()
{
  gl_Position = f();
}

... and m.frag:

#version 330 core

layout(location = 0) out vec4 out_rgba;

void
main()
{
  out_rgba = vec4(1.0);
}

Validating with glslangValidator -l m.vert m.frag gives:

m.vert
m.frag

Linked vertex stage:


Linked fragment stage:

No error is indicated, despite the definition for f not having been given anywhere. Is there a way to get the validator to catch errors of this type? In C-like languages, that would obviously become a link-time error.

For example,

#if 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

Crashes glslangValidator with a segmentation fault.

Can anybody shed any light on why there's an empty unistd.h in glslang/MachineIndependent/ ? From a quick grep, I don't see that actually get used anywhere in glslang.

This is causing problems when being built into a test app in an environment where we can't use the CMake build system. This empty file is then getting picked up due to include path sharing, breaking other code which needs a real unistd.h.

Thanks,
//Mark

I noticed while changing the CMakeLists.txt files for the osinclude pull request that there is this line in the root CMakeLists.txt:

    set(CMAKE_GENERATOR_TOOLSET "v110" CACHE STRING "Platform Toolset" FORCE)

I'm not sure why this toolset is important (is it a minimum, since C++11 features are used that don't work in v100? or is using a higher toolset undesirable?), but it means a bit of fiddling locally if you don't have 2012 installed.

Perhaps there is a way to check in CMake if that toolset exists before forcing it?

Instead of 0x08 ConstOffset modifier, 0x10 Offset is emitted, which is incorrect since textureLodOffset must have a constant expression offset.

The input float a[];float b = float(a) (casting from an array with no explicit size) causes glslangValidator to fail an assertion (isExplicitlySizedArray()) and abort through libc instead of printing an error.

This seems like a pretty glaring omission. I took a look at the code and it doesn't look like this should take much time for someone who knows what they're doing with it. However, my knowledge of the glslang code-base is slim at best and I wasn't seeing exactly how it should be added.

A loop merge block (loop header) would need some special setup before any of its spirv instructions is parsed.
Meaning you would have, in the label instruction processing of this block, to parse ahead to look up for a loop merge instruction and then know if you have to emit the loop setup machine instructions.
Then, the 'one pass transform' would not be "per spirv instruction" but "per spirv block". Am I wrong? Do I miss something?

#endif
#endif
#endif
#endif
#endif
#endif
#endif

#if
#else

and a number of similar inputs exercise a segmentation fault bug in glslang::TPpContext::tStringInput::scan.

struct s {
  vec2 b[];
};

void a(){
  s(0)

I tried this with the latest master, and this version of the bug still aborts the process.

The GLSL specification (this text appears to be the same in all versions of the specification) says this:

#define and #undef functionality are defined as is standard for C++ preprocessors for macro definitions
both with and without macro parameters.

So it follows that using defined as a macro name should throw an error, as is standard for C and C++ preprocessors, defined is not a valid macro name.
For example, clang gives:

test.c:1:9: error: 'defined' cannot be used as a macro name
#define defined definer
        ^

and GCC gives almost identically:

test.c:1:9: error: "defined" cannot be used as a macro name
 #define defined definer
         ^

For purposes of readability and potentially performance, it would be nice to avoid the shadowed "params" variable where appropriate.

Typical code-gen when passing simple in arguments to functions get first copyed to a separate variable (via OpStore).

#version 310 es

vec4 foo(float v)
{
    return vec4(v);
}

void main()
{
    float v = 10.0;
    gl_Position = foo(v);
}

Compiles into

         4(main):           2 Function None 3
               5:             Label
           16(v):      7(ptr) Variable Function
       23(param):      7(ptr) Variable Function
                              Store 16(v) 17
              24:    6(float) Load 16(v)
                              Store 23(param) 24
              25:    8(fvec4) FunctionCall 11(foo(f1;) 23(param)
              27:     26(ptr) AccessChain 20 22
                              Store 27 25
                              Return
                              FunctionEnd

There is no need for this param variable, and it only adds clutter when disassembling and reading the SPIR-V.

For out/inout arguments, param shadowing makes more sense though due to GLSL's weird semantics with out/inout arguments.

Builtin functions imageXXX are not supported by SPV builder.

revision.h is intended for auto-updated versioning on each change, but this is not working under GitHub; a mechanism needs to be put in place for it.

glslangValidator is becoming a SPIR-V front end, as well as a validator. Khronos decided it should be name just "glslang". So this change in the built executable name will likely occur at some point.

Are there any concerns about doing this?

Consider the following shader:

layout(location = 0) in vec2 data;
layout(set = 0, binding = 0, std140) buffer Storage {
   vec4 arr[];
} ssbo;

void main()
{
   if (gl_VertexID % 2 == 0) {
      ssbo.arr[gl_VertexID / 2].xz = data;
   } else {
      ssbo.arr[gl_VertexID / 2].yw = data;
   }
}

The two SSBO stores should only alter the components of the SSBO specified by the swizzle. I can invoke this shader as a SIMD8 shader on a Broadwell in such a way that one SIMD8 shader invocation will take vertices with an even gl_VertexID and another will take the odd ones. This means that one EU (execution unit) will only execute the first case and the other will only execute the second case. GLSLang converts this shader to the following SPIR-V:

                              Source GLSL 430
                              SourceExtension  "GL_GOOGLE_cpp_style_line_directive"
                              SourceExtension  "GL_GOOGLE_include_directive"
                              Capability Shader
               1:             ExtInstImport  "GLSL.std.450"
                              MemoryModel Logical GLSL450
                              EntryPoint Vertex 4  "main"
                              Name 4  "main"
                              Name 8  "gl_VertexID"
                              Name 20  "Storage"
                              MemberName 20(Storage) 0  "arr"
                              Name 22  "ssbo"
                              Name 27  "data"
                              Name 40  "gl_InstanceID"
                              Decorate 8(gl_VertexID) BuiltIn VertexId
                              Decorate 19 ArrayStride 16
                              MemberDecorate 20(Storage) 0 Offset 0
                              Decorate 20(Storage) BufferBlock
                              Decorate 22(ssbo) DescriptorSet 0
                              Decorate 22(ssbo) Binding 0
                              Decorate 27(data) Location 0
                              Decorate 40(gl_InstanceID) BuiltIn InstanceId
               2:             TypeVoid
               3:             TypeFunction 2
               6:             TypeInt 32 1
               7:             TypePointer Input 6(int)
  8(gl_VertexID):      7(ptr) Variable Input
              10:      6(int) Constant 2
              12:      6(int) Constant 0
              13:             TypeBool
              17:             TypeFloat 32
              18:             TypeVector 17(float) 4
              19:             TypeRuntimeArray 18(fvec4)
     20(Storage):             TypeStruct 19
              21:             TypePointer Uniform 20(Storage)
        22(ssbo):     21(ptr) Variable Uniform
              25:             TypeVector 17(float) 2
              26:             TypePointer Input 25(fvec2)
        27(data):     26(ptr) Variable Input
              29:             TypePointer Uniform 18(fvec4)
40(gl_InstanceID):      7(ptr) Variable Input
         4(main):           2 Function None 3
               5:             Label
               9:      6(int) Load 8(gl_VertexID)
              11:      6(int) SMod 9 10
              14:    13(bool) IEqual 11 12
                              SelectionMerge 16 None
                              BranchConditional 14 15 33
              15:               Label
              23:      6(int)   Load 8(gl_VertexID)
              24:      6(int)   SDiv 23 10
              28:   25(fvec2)   Load 27(data)
              30:     29(ptr)   AccessChain 22(ssbo) 12 24
              31:   18(fvec4)   Load 30
              32:   18(fvec4)   VectorShuffle 31 28 4 1 5 3
                                Store 30 32
                                Branch 16
              33:               Label
              34:      6(int)   Load 8(gl_VertexID)
              35:      6(int)   SDiv 34 10
              36:   25(fvec2)   Load 27(data)
              37:     29(ptr)   AccessChain 22(ssbo) 12 35
              38:   18(fvec4)   Load 37
              39:   18(fvec4)   VectorShuffle 38 36 0 4 2 5
                                Store 37 39
                                Branch 16
              16:             Label
                              Return
                              FunctionEnd

As you can see, in both cases, it does a load, a vector-shuffle, and a store. This creates a race condition where both EU's could, in theory, do the load at the same time, execute the vector-shuffle, and then both store all 4 components. Whoever gets to the store first wins the race and the other thread's data gets stompped.

We found a similar bug in the GLSL compiler in upstream mesa recently which was causing it to fail compute shader conformance tests.

The MissingFunctionality() function in SPIRV/SpvBuilder.cpp here calls exit(1) unconditionally. This is a problem if glslang is used as a library and not as part of the standalone validator, because then there's no hope of recovering or continuing execution if the application can handle SPIR-V failing to compile.

There's also at least one case - I'd guess more - where if this exit() call is removed then the code will crash later on. I ran into this in GlslangToSpv.cpp around line 808:

    case glslang::EOpFunctionCall:
    {
        if (node->isUserDefined())
            result = handleUserFunctionCall(node);
        else
            result = handleBuiltInFunctionCall(node);

        if (! result) {
            spv::MissingFunctionality("glslang function call");
            glslang::TConstUnionArray emptyConsts;
            int nextConst = 0;
            result = createSpvConstant(node->getType(), emptyConsts, nextConst);
        }
        builder.clearAccessChain();
        builder.setAccessChainRValue(result);

        return false;
    }

Where setAccessChainRValue ends up accessign a NULL pointer when it calls getTypeId().

A parameter to pass preprocessor directives is necessary. For example:
glslangValidator test.frag -D __LINUX__ or
glslangValidator test.frag -D __ANDROID__

float a[];
float b = vec4(a)

Related to #26, this is not detected before it gets to computeNumComponents and the assertion fails.

struct s {
  const vec2 b[];
};

void a() {
  s t;
  (t=0).b[0]

Interesting segmentation fault also related to struct constructors.

What should be the minimum version of MSVC required for glslang?

I'd been setting the bar at 2012 to be conservative across a broad base, but many submissions require 2013, which I then fix to work on 2012. (2012 supports a broad set of C++11, but not all those supported by 2013.)

Any input on a required minimum version?

#endif
#endif
#endif
#endif
#endif
#endif
#endif
#endif

, Or any other sequence of eight or more unmatched #endif directives, triggers a segmentation fault in glslang::TParseContext::outputMessage.

Currently a bit messy in my opinion and can be unclear to those not familiar (what does +/- mean? missing features, bugs?). Would be easier to keep up to date and see who's working on what.

When outputting SPIRV with GLSL code like

uniform writeonly image2D foo;

the SPIRV code does not emit decorations for this.

The input uniform m{float f;};void n(){0=mat3(m) when interpereted as a fragment shader causes glslangValidator to crash with SIGSEGV in computeNumComponents.

Would like to move to enum-based operators for texturing and image operations.

Currently, almost all built-in functions are mapped to the TOperator enum, and can be dealt with numerically. However, this is not true for texturing and imaging operations. It would be better to do all this consistently and eliminate text-based comparisons.

As consumers of glslang would break and need to adapt to this, this is a place to gather input. (As is the Khronos public bug 1346.)

See

const bool PureOperatorBuiltins = false;  // could break backward compatibility; pending feedback

in Initialize.cpp, where this is currently set up, but turned off.

Return statements in loops inside a void-returning function generates a branch instruction which targets
the function terminating return block.
It means there is a branch from inside a nested flow control
to an outside return block. There is no other other way than performing a full 2 pass transform in order to know if a branch targets a return block.
Additionnaly, if a return block in not in the nested flow control, we cannot use statically allocated hardware resource.
Do I miss something?