Add Declaration Nodes to AST for Better Debug Info

[qingyuanzNV]

Glslang is having trouble to generate good debug information for variables because we don't have AST representation for variable definitions. The variables are generated at the site of its first use, where some of the original declaration information is already lost.

In https://github.com/KhronosGroup/glslang/pull/3401, I attempted to correct the global variable line information by adding a TSourceLoc to track the original definition location in TIntermSource. But this approach doesn't fix a broader issue that some variables may be stripped away because of constant folding or unused symbol.

We could probably do better with an optional toggle to allow glslang to build AST nodes for variable declaration. A proof-of-concept implementation is at here. This should be optional to 1) don't slow down standard library compilation 2) don't break existing programs that work on glslang AST

Given an example shader program:

out float outz;

and

#version 450
#extension GL_GOOGLE_include_directive : require
#include "test.h"

// CHECK: COMMENT ONE!
const float one = 1;
in float inx, iny;
in float inz;

float bar(int x) {
    return x + one;
}

float foo(float x, const float y) {
    return x + y;
}

void main() {
    const int localOne = 1;
    outz = foo(inx, iny) + bar(localOne);
}

We'll create an AST

Shader version: 450
Requested GL_GOOGLE_cpp_style_line_directive
Requested GL_GOOGLE_include_directive
gl_FragCoord origin is upper left
0:? Sequence
0:2  Sequence
0:?     Decl: outz
0:6  Sequence
0:?     Decl: one
0:7  Sequence
0:?     Decl: inx
0:?     Decl: iny
0:8  Sequence
0:?     Decl: inz
0:10  Function Definition: bar(i1; ( global highp float)
0:10    Function Parameters:
0:10      'x' ( in highp int)
0:11    Sequence
0:11      Branch: Return with expression
0:11        add ( temp highp float)
0:11          Convert int to float ( temp highp float)
0:11            'x' ( in highp int)
0:11          Constant:
0:11            1.000000
0:14  Function Definition: foo(f1;f1; ( global highp float)
0:14    Function Parameters:
0:14      'x' ( in highp float)
0:14      'y' ( const (read only) highp float)
0:15    Sequence
0:15      Branch: Return with expression
0:15        add ( temp highp float)
0:15          'x' ( in highp float)
0:15          'y' ( const (read only) highp float)
0:18  Function Definition: main( ( global void)
0:18    Function Parameters:
0:19    Sequence
0:19      Sequence
0:?         Decl: localOne
0:20      move second child to first child ( temp highp float)
0:20        'outz' ( out highp float)
0:20        add ( temp highp float)
0:20          Function Call: foo(f1;f1; ( global highp float)
0:20            'inx' ( smooth in highp float)
0:20            'iny' ( smooth in highp float)
0:20          Function Call: bar(i1; ( global highp float)
0:20            Constant:
0:20              1 (const int)
0:?   Linker Objects
0:?     'outz' ( out highp float)
0:?     'one' ( const highp float)
0:?       1.000000
0:?     'inx' ( smooth in highp float)
0:?     'iny' ( smooth in highp float)
0:?     'inz' ( smooth in highp float)

resulting in SPIR-V (with non-semantic debug):

; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 11
; Bound: 124
; Schema: 0
               OpCapability Shader
               OpExtension "SPV_KHR_non_semantic_info"
          %3 = OpExtInstImport "NonSemantic.Shader.DebugInfo.100"
          %4 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Fragment %main "main" %outz %inx %iny %inz
               OpExecutionMode %main OriginUpperLeft
          %1 = OpString "test.frag"
          %2 = OpString "./test.h"
          %9 = OpString "uint"
         %18 = OpString "int"
         %23 = OpString "float"
         %30 = OpString "bar"
         %33 = OpString "// OpModuleProcessed auto-map-bindings
// OpModuleProcessed auto-map-locations
// OpModuleProcessed client vulkan100
// OpModuleProcessed target-env vulkan1.0
// OpModuleProcessed entry-point main
#line 1
#version 450
#extension GL_GOOGLE_include_directive : require
#include \"test.h\"

// CHECK: COMMENT ONE!
const float one = 1;
in float inx, iny;
in float inz;

float bar(int x) {
    return x + one;
}

float foo(float x, const float y) {
    return x + y;
}

void main() {
    const int localOne = 1;
    outz = foo(inx, iny) + bar(localOne);
}"
         %41 = OpString "x"
         %51 = OpString "foo"
         %59 = OpString "y"
         %61 = OpString "main"
         %67 = OpString "

out float outz;"
         %71 = OpString "outz"
         %76 = OpString "one"
         %82 = OpString "inx"
         %85 = OpString "iny"
         %89 = OpString "inz"
        %113 = OpString "localOne"
               OpSourceExtension "GL_GOOGLE_cpp_style_line_directive"
               OpSourceExtension "GL_GOOGLE_include_directive"
               OpName %main "main"
               OpName %bar_i1_ "bar(i1;"
               OpName %x "x"
               OpName %foo_f1_f1_ "foo(f1;f1;"
               OpName %x_0 "x"
               OpName %y "y"
               OpName %outz "outz"
               OpName %inx "inx"
               OpName %iny "iny"
               OpName %inz "inz"
               OpName %param "param"
               OpName %param_0 "param"
       %void = OpTypeVoid
          %6 = OpTypeFunction %void
       %uint = OpTypeInt 32 0
    %uint_32 = OpConstant %uint 32
     %uint_6 = OpConstant %uint 6
     %uint_0 = OpConstant %uint 0
         %10 = OpExtInst %void %3 DebugTypeBasic %9 %uint_32 %uint_6 %uint_0
     %uint_3 = OpConstant %uint 3
          %7 = OpExtInst %void %3 DebugTypeFunction %uint_3 %void
        %int = OpTypeInt 32 1
     %uint_4 = OpConstant %uint 4
         %19 = OpExtInst %void %3 DebugTypeBasic %18 %uint_32 %uint_4 %uint_0
%_ptr_Function_int = OpTypePointer Function %int
      %float = OpTypeFloat 32
         %24 = OpExtInst %void %3 DebugTypeBasic %23 %uint_32 %uint_3 %uint_0
         %25 = OpTypeFunction %float %_ptr_Function_int
         %26 = OpExtInst %void %3 DebugTypeFunction %uint_3 %24 %19
         %32 = OpExtInst %void %3 DebugSource %1 %33
    %uint_10 = OpConstant %uint 10
     %uint_1 = OpConstant %uint 1
     %uint_2 = OpConstant %uint 2
         %35 = OpExtInst %void %3 DebugCompilationUnit %uint_1 %uint_4 %32 %uint_2
         %31 = OpExtInst %void %3 DebugFunction %30 %26 %32 %uint_10 %uint_0 %35 %30 %uint_3 %uint_10
         %40 = OpExtInst %void %3 DebugLocalVariable %41 %19 %32 %uint_10 %uint_0 %31 %uint_4 %uint_1
         %43 = OpExtInst %void %3 DebugExpression
%_ptr_Function_float = OpTypePointer Function %float
         %45 = OpTypeFunction %float %_ptr_Function_float %float
         %46 = OpExtInst %void %3 DebugTypeFunction %uint_3 %24 %24 %24
    %uint_14 = OpConstant %uint 14
         %52 = OpExtInst %void %3 DebugFunction %51 %46 %32 %uint_14 %uint_0 %35 %51 %uint_3 %uint_14
         %56 = OpExtInst %void %3 DebugLocalVariable %41 %24 %32 %uint_14 %uint_0 %52 %uint_4 %uint_1
         %58 = OpExtInst %void %3 DebugLocalVariable %59 %24 %32 %uint_14 %uint_0 %52 %uint_4 %uint_2
    %uint_18 = OpConstant %uint 18
         %62 = OpExtInst %void %3 DebugFunction %61 %7 %32 %uint_18 %uint_0 %35 %61 %uint_3 %uint_18
         %66 = OpExtInst %void %3 DebugSource %2 %67
%_ptr_Output_float = OpTypePointer Output %float
       %outz = OpVariable %_ptr_Output_float Output
     %uint_8 = OpConstant %uint 8
         %70 = OpExtInst %void %3 DebugGlobalVariable %71 %24 %66 %uint_2 %uint_0 %35 %71 %outz %uint_8
    %float_1 = OpConstant %float 1
         %75 = OpExtInst %void %3 DebugGlobalVariable %76 %24 %32 %uint_6 %uint_0 %35 %76 %float_1 %uint_8
     %uint_7 = OpConstant %uint 7
%_ptr_Input_float = OpTypePointer Input %float
        %inx = OpVariable %_ptr_Input_float Input
         %81 = OpExtInst %void %3 DebugGlobalVariable %82 %24 %32 %uint_7 %uint_0 %35 %82 %inx %uint_8
        %iny = OpVariable %_ptr_Input_float Input
         %84 = OpExtInst %void %3 DebugGlobalVariable %85 %24 %32 %uint_7 %uint_0 %35 %85 %iny %uint_8
        %inz = OpVariable %_ptr_Input_float Input
         %88 = OpExtInst %void %3 DebugGlobalVariable %89 %24 %32 %uint_8 %uint_0 %35 %89 %inz %uint_8
    %uint_11 = OpConstant %uint 11
    %uint_15 = OpConstant %uint 15
    %uint_19 = OpConstant %uint 19
      %int_1 = OpConstant %int 1
        %112 = OpExtInst %void %3 DebugLocalVariable %113 %19 %32 %uint_19 %uint_0 %62 %uint_4
    %uint_20 = OpConstant %uint 20
               OpLine %1 18 11
       %main = OpFunction %void None %6
         %16 = OpLabel
      %param = OpVariable %_ptr_Function_float Function
    %param_0 = OpVariable %_ptr_Function_int Function
         %64 = OpExtInst %void %3 DebugScope %35
         %65 = OpExtInst %void %3 DebugLine %66 %uint_2 %uint_2 %uint_0 %uint_0
         %73 = OpExtInst %void %3 DebugLine %32 %uint_6 %uint_6 %uint_0 %uint_0
         %77 = OpExtInst %void %3 DebugLine %32 %uint_7 %uint_7 %uint_0 %uint_0
         %86 = OpExtInst %void %3 DebugLine %32 %uint_8 %uint_8 %uint_0 %uint_0
        %107 = OpExtInst %void %3 DebugFunctionDefinition %62 %main
        %108 = OpExtInst %void %3 DebugScope %62
        %109 = OpExtInst %void %3 DebugLine %32 %uint_19 %uint_19 %uint_0 %uint_0
        %114 = OpExtInst %void %3 DebugValue %112 %int_1 %43
        %115 = OpExtInst %void %3 DebugLine %32 %uint_20 %uint_20 %uint_0 %uint_0
        %117 = OpLoad %float %iny
        %119 = OpLoad %float %inx
               OpStore %param %119
        %120 = OpFunctionCall %float %foo_f1_f1_ %param %117
               OpStore %param_0 %int_1
        %122 = OpFunctionCall %float %bar_i1_ %param_0
        %123 = OpFAdd %float %120 %122
               OpStore %outz %123
               OpReturn
               OpFunctionEnd
               OpLine %1 10 16
    %bar_i1_ = OpFunction %float None %25
          %x = OpFunctionParameter %_ptr_Function_int
         %29 = OpLabel
         %38 = OpExtInst %void %3 DebugScope %31
         %39 = OpExtInst %void %3 DebugLine %32 %uint_10 %uint_10 %uint_0 %uint_0
         %42 = OpExtInst %void %3 DebugDeclare %40 %x %43
         %90 = OpExtInst %void %3 DebugFunctionDefinition %31 %bar_i1_
         %91 = OpExtInst %void %3 DebugScope %31
         %92 = OpExtInst %void %3 DebugLine %32 %uint_11 %uint_11 %uint_0 %uint_0
         %94 = OpLoad %int %x
         %95 = OpConvertSToF %float %94
         %96 = OpFAdd %float %95 %float_1
               OpReturnValue %96
               OpFunctionEnd
               OpLine %1 14 33
 %foo_f1_f1_ = OpFunction %float None %45
        %x_0 = OpFunctionParameter %_ptr_Function_float
          %y = OpFunctionParameter %float
         %50 = OpLabel
         %54 = OpExtInst %void %3 DebugScope %52
         %55 = OpExtInst %void %3 DebugLine %32 %uint_14 %uint_14 %uint_0 %uint_0
         %57 = OpExtInst %void %3 DebugDeclare %56 %x_0 %43
         %60 = OpExtInst %void %3 DebugValue %58 %y %43
         %99 = OpExtInst %void %3 DebugFunctionDefinition %52 %foo_f1_f1_
        %100 = OpExtInst %void %3 DebugScope %52
        %101 = OpExtInst %void %3 DebugLine %32 %uint_15 %uint_15 %uint_0 %uint_0
        %103 = OpLoad %float %x_0
        %104 = OpFAdd %float %103 %y
               OpReturnValue %104
               OpFunctionEnd

The prototype needs some refinement, but we could clearly see we are now correctly generating debug information for global variables and global/local constants.

[arcady-lunarg]

I think this is a good idea and will also help with various other DebugInfo issues we have had. As you say, it makes sense to limit this to variables explicitly declared by the user, given that this will mostly be used for DebugInfo. Indeed, maybe it makes sense to only have those nodes generated when the user requests generation of nonsemantic debuginfo.

[qingyuanzNV]

Would like to collect opinions about how the declaration should be represented in the AST. The current implementation looks like:

class TIntermDecl : public TIntermNode {
    TIntermSymbol* declSymbol; // a dummy symbol access
    TIntermNode* initNode; // initializer expression
};

I'm a little concerned about whether we could reuse the TIntermSymbol here. It could significantly simplify the implementation, but I'm not sure if this would break some traversal implementation.