shader-slang / slang

Making it easier to work with shaders
MIT License
1.78k stars 159 forks source link

No debuginfo generated for buffer references #4077

Closed arcady-lunarg closed 1 month ago

arcady-lunarg commented 2 months ago

In HLSL, using the vk::BufferPointer syntax/, I don't see any debug info emitted for buffer pointer struct members when using the -g3 options. There's no OpDebugTypePointer for the buffer reference and no OpDebugTypeStruct for the struct being pointed to, and indeed I don't see any code in slang to emit the OpDebugTypePointer instruction. With GLSL the layout(buffer_reference) syntax doesn't seem to supported at all.

csyonghe commented 2 months ago

Currently pointers are emitted as normal 64bit integers in our debug info because we weren't sure if the downstream tools supported OpDebugTypePointer. We can added this if required.

csyonghe commented 2 months ago

Also our GLSL support is experimental and is provided only to help developers transition from existing/legacy GLSL codebase. The GLSL support isn't meant to be complete or kept up-to-date with the GLSL spec.

jkwak-work commented 1 month ago

Can I ask a simple repro case for the issue?

csyonghe commented 1 month ago
struct Data
{
     float4x3 mat;
     float4 vec;
     float3 arr[5];
     Data* next;
}

ConstantBuffer<Data> cData;
RWStructuredBuffer<float> result;

[numthread(1,1,1)]
void main()
{
      result[0] = cData.next->arr[2].x + cData.next->vec.y;
}

Run with slangc test.slang -o test.spv -g2. We should be producing an OpDebugTypePointer member in the OpDebugTypeStruct for Data.

jkwak-work commented 1 month ago

I have been having troubles while making a simple repro case. It turned out that slangc is crashing on my repro cases. The crash is caused by a stack overflow while calling "isSimpleDataType()" recursively forever.

I am debugging the problem.

jkwak-work commented 1 month ago

I like to leave an update.

I believe I fixed the crashing issue I mentioned on my previous comment. It had something to do with the nature of recursiveness of memory pointer. When there is a member variable whose type is a pointer to the struct it belongs to, it will look like the following.

struct Data
{
    float value;
    Data *next;
};

When we visit "Data", it iterates all members one by one. When it encounter a pointer, it, kind of, dereferences the pointer in an attempt to figure out what kind of struct it is. Then it iterates all members and encounter the pointer again and it recursively repeated until it ran out of the stack memory. I believe I have a fix for this.

The next problem I am dealing with has a similar nature. If a given struct is like the following, its SPIR-V code will look like the one shown below,

struct Data
{
    float4 value;
};

%1 = DebugTypeBasic "float" ... // define "float"
%2 = DebugTypeVector %1 "4" ... // define "vector<float,4>"
%3 = DebugTypeMember "value" %2 ... // define "vector<float,4> value" as a member variable
%4 = DebugTypeComposite "Data" ... %3 // define "Data" as a struct with a member variable defined in %3.

When I have a pointer as a member, it changes to the following,

struct Data
{
    float4 value;
    Data *next;
};

%1 = DebugTypeBasic "float" ... // define "float"
%2 = DebugTypeVector %1 "4" ... // define "vector<float,4>".
%3 = DebugTypeMember "value" %2 ... // define "vector<float,4> value" as a member variable.
%4 = DebugTypeComposite "Data" ... %3 // define "Data" as a struct with a member variable defined in %3.
%5 = DebugTypePointer %4 // define a pointer type, "Data*" using "%4".
%6 = DebugTypeMember "next" %5 // define "Data* next" as a member variable.
%7 = DebugTypeComposite "Data" ... %3 %6// define "Data" as a struct with member variables defined in %3 and %6.

In this case, however, "%4" should go away, because it conflicts with "%7". But "%4" is needed to define the pointer type at "%5". The thing I need to figure out is how to do some kind of forward declaration of the type "Data" before DebugTypePointer.

When I asked chatGPU, the answer was

forward declare Data: Create a placeholder for Data without defining its members initially

I will try if it works.

arcady-lunarg commented 1 month ago

Currently you can't have forward references in nonsemantic instructions, however there is an extension that will be released soon that will allow it.

jkwak-work commented 1 month ago

Thank you for the reply. When I tried with spirv-as.exe, it looked like working, but the pointer might be pointing to a wrong struct in that case.

The document says the following about "member" in DebugTypeComposite but I am not sure what it means...

Members must be the <id>s of DebugTypeMember, DebugFunction, or DebugTypeInheritance. This could be a forward reference.

If forward-declaration is not possible, I will skip this specific case and focus more on other pointer types that don't require a forward-declaration.

jkwak-work commented 1 month ago

It looks like DebugTypePointer works only for DebugTypeBasic and it doesn't work for DebugTypeComposite. When I tried, glslangValidator.exe gave me an error saying the DebugTypePointer requires DebugTypeBasic.

jkwak-work commented 1 month ago

I have something to clarify. I found that "DebugTypeXXX" like DebugTypeBasic or DebugTypeMember are emitted only when the shader creates an object for the struct. If a struct is used only as a pointer, "DebugTypeXXX" are not emitted.

The following example shows the problem.

RWStructuredBuffer<float> outputBuffer;

struct LinkedNode
{
    float value;
    float *pValue;
    LinkedNode *pNext;
};

float test(LinkedNode *pNode)
{
    LinkedNode *pNodeNext = pNode->pNext;
    return *(pNode->pValue) + pNodeNext->value;
}

cbuffer Constants
{
    LinkedNode *head;
};

[numthreads(1,1,1)]
void computeMain()
{
    LinkedNode node; // <== THIS IS REQUIRED TO EMIT DebugTypeXXX FOR LinkedNode
    outputBuffer[0] = test(head);
}

When I create a dummy variable, "node", it emites "DebugTypeXXX". But without the variable, it doesn't emit any of "DebugTypeXXX", although it emits "DebugValue". @csyonghe , is this an expected behavior?

csyonghe commented 1 month ago

I am not sure what you refer to "does not emit DebugValue". The intention here is that if there is no variable, then we shouldn't be emitting DebugValue, because the only case it make sense to have a DebugValue inst is when the user is changing the value for a variable.

The user can only inspect values of variables inside debugger, so it doesn't make sense to create DebugValue for temporary values that is not visible from the debugger.

jkwak-work commented 1 month ago

It seems like there was a misunderstand.

DebugValue are emitted properly. But "DebugTypeXXX" are not emitted until the struct is used to create an object. My question is when a struct is used as a pointer, shouldn't it still emit information about the struct?

  LinkedNode obj; // <== this triggers the emitting DebugTypeXXX for the struct and member variables.
  LinkedNode* obj; // <== This doesn't trigger it, but we may need to, isn't it?
csyonghe commented 1 month ago

In your second case, because the debug type for obj is just uint64, there is no need to represent LinkedNode in the debug type hierarchy, right? Since there are no debugValue insts that sets a value of LInkedNode type, we shouldn't be generating the DebugType for LinkedNode.

jkwak-work commented 1 month ago

I see. Thanks for clarifying it.