--auto-map-locations gets disabled when linking mutiple shaders

[bpeel]

If you pass in multiple shader sources on the command line then --auto-map-locations gets disabled.

It looks like a bunch of configuration state from the command line, including --aml, is set on the TIntermediate for a shader in CompileAndLinkShaderUnits in StandAlone.cpp. However in TProgram::linkStage when there are multiple compilation units it creates a new TIntermediate to compile each one and this configuration state is not copied over.

Perhaps the configuration state within TIntermediate could be stored in a struct with a convenient way to copy it to a new intermediate. Or maybe global configuration state from the command line should be stored somewhere else where it is globally accessible.

[johnkslang]

We will want to discuss multiple stages independently from multiple compilation units for the same stage. There are definite holes, depending on what you want to do, but it will help to be clear which category:

• validation-only (no SPIR-V generation) of multiple source files for the same pipeline [semantic checking is only partially implemented]
• multiple source files, linked into a single stage, making a single-entrypoint SPIR-V module [numerous detail-level issues]
• multiple source files for multiple stages, to make a SPIR-V module with multiple entry points [not supported at all, and should be handled by separate compilation and a module merger]

[bpeel]

This is the second category, ie linking multiple compilation units together into a single stage. For example, if I have this in one file called part1.frag:

#version 430
out vec4 color_out;
void main()
{
        color_out = vec4(1.0);
}

And then I have this almost empty second file in part2.frag:

#version 430

And then I link them together into a single fragment module like this:

glslangValidator --aml -V part1.frag part2.frag

Then the color_out variable doesn’t get a Location decoration. If I leave out the part2.frag argument then it does.

[johnkslang]

I am looking now at multiple compilation units for compiling to SPIR-V rather than just validating. There are a few fundamental issues to overcome.

[johnkslang]

FYI: #1446 Includes the hard part for fixing up symbol IDs.

[johnkslang]

I've considered a few paths here, and it seems the most general case (due to API usage, not to command-line usage) is to keep all this state per compilation unit, as it currently is, and just include the missing ones in the merge.

There is a PR on the way that does this, but does not yet have a broad test suite.

[infapi00]

Hi, I found some crashes, and doing git-bisect, they started with this commit:

     commit b617e14acb3f724ff9d71bf6c5681da3b51e0fcf
     Author: John Kessenich <cepheus@frii.com>
     Date:   Thu Jul 19 23:10:32 2018 -0600

         Link: Merge all the settings in TIntermediate.

    Fixes #1309.

So for example, this crash if you use the following two vertex shaders:

1.vert

#version 450

out blk {
  vec4 foo;
} inst[2][5];

void f()
{
  inst[1][4].foo = vec4(1.0);
}

And 2.vert:

#version 450

out blk {
  vec4 foo;
} inst[2][5];

void f();

void main()
{
  f();
  inst[0][4].foo = vec4(1.0);
}

We get a crash when using --aml:

$ glslangValidator --aml -G /tmp/1.vert /tmp/2.vert
/tmp/1.vert
/tmp/2.vert
Segmentation fault

Without --aml, it just fails, due missing locations.

And just in case it is interesting, the bt I got is this:

(gdb) bt
#0 0x00005555556efb2e in (anonymous namespace)::TGlslangToSpvTraverser::visitBinary (this=0x7fffffffc2d0, node=0x55555643e830) at /home/infapi00/mesa/source/glslang/SPIRV/GlslangToSpv.cpp:1507
#1 0x00005555556dcea6 in glslang::TIntermBinary::traverse (this=0x55555643e830, it=0x7fffffffc2d0) at /home/infapi00/mesa/source/glslang/glslang/MachineIndependent/IntermTraverse.cpp:85
#2 0x00005555556ef3ea in (anonymous namespace)::TGlslangToSpvTraverser::visitBinary (this=0x7fffffffc2d0, node=0x55555643ed40) at /home/infapi00/mesa/source/glslang/SPIRV/GlslangToSpv.cpp:1447
#3 0x00005555556dcea6 in glslang::TIntermBinary::traverse (this=0x55555643ed40, it=0x7fffffffc2d0) at /home/infapi00/mesa/source/glslang/glslang/MachineIndependent/IntermTraverse.cpp:85
#4 0x00005555556dd73a in glslang::TIntermAggregate::traverse (this=0x55555643e1e0, it=0x7fffffffc2d0) at /home/infapi00/mesa/source/glslang/glslang/MachineIndependent/IntermTraverse.cpp:168
#5 0x00005555556dd73a in glslang::TIntermAggregate::traverse (this=0x55555643edf0, it=0x7fffffffc2d0) at /home/infapi00/mesa/source/glslang/glslang/MachineIndependent/IntermTraverse.cpp:168
#6 0x00005555556fa241 in (anonymous namespace)::TGlslangToSpvTraverser::visitFunctions (glslFunctions=..., this=0x7fffffffc2d0) at /home/infapi00/mesa/source/glslang/SPIRV/GlslangToSpv.cpp:3504
#7 (anonymous namespace)::TGlslangToSpvTraverser::visitAggregate (this=0x7fffffffc2d0, visit=, node=) at /home/infapi00/mesa/source/glslang/SPIRV/GlslangToSpv.cpp:1820
#8 0x00005555556dd76e in glslang::TIntermAggregate::traverse (this=0x55555641a350, it=0x7fffffffc2d0) at /home/infapi00/mesa/source/glslang/glslang/MachineIndependent/IntermTraverse.cpp:152
#9 0x00005555556ff1dc in glslang::GlslangToSpv (intermediate=..., spirv=std::vector of length 0, capacity 0, logger=0x7fffffffccd0, options=0x7fffffffccab)
at /home/infapi00/mesa/source/glslang/SPIRV/GlslangToSpv.cpp:7305
#10 0x0000555555605815 in CompileAndLinkShaderUnits (compUnits=std::vector of length 2, capacity 2 = {...}) at /home/infapi00/mesa/source/glslang/StandAlone/StandAlone.cpp:986
#11 0x0000555555608ab1 in CompileAndLinkShaderFiles (Worklist=...) at /home/infapi00/mesa/source/glslang/StandAlone/StandAlone.cpp:1066
#12 0x0000555555609502 in singleMain () at /home/infapi00/mesa/source/glslang/StandAlone/StandAlone.cpp:1137
#13 0x00007ffff711b1c1 in __libc_start_main (main=0x5555556028b0 <main(int, char**)>, argc=5, argv=0x7fffffffd128, init=, fini=, rtld_fini=,
stack_end=0x7fffffffd118) at ../csu/libc-start.c:308
#14 0x000055555560471a in _start ()

Not sure if I should open a new issue, or let this comment here to re-open this one.

[infapi00]

I have just tested master, and it seems that this is not a issue anymore. I didn't check when this stopped to be a problem, but I don't think that we need to really check that. I think that it is ok to close the issue.