Closed bpeel closed 5 years ago
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:
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.
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.
FYI: #1446 Includes the hard part for fixing up symbol IDs.
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.
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.
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.
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 inCompileAndLinkShaderUnits
inStandAlone.cpp
. However inTProgram::linkStage
when there are multiple compilation units it creates a newTIntermediate
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.