Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPIR-V] Shader failes to compile when activating debug infos #4911

Closed
Clothoid1 opened this issue Jan 4, 2023 · 7 comments
Closed

[SPIR-V] Shader failes to compile when activating debug infos #4911

Clothoid1 opened this issue Jan 4, 2023 · 7 comments
Assignees
Labels
bug Bug, regression, crash spirv Work related to SPIR-V

Comments

@Clothoid1
Copy link

Clothoid1 commented Jan 4, 2023

The following reproducer does not compile, leading to the error message:

fatal error : generated SPIR-V is invalid: ID '52[%52]' has not been defined
1>  %51 = OpExtInst %void %1 DebugTypeFunction %uint_3 %52 %48 %49
1>
1>note: please file a bug report on https://github.com/Microsoft/DirectXShaderCompiler/issues with source code if possible

Reproducer:

StructuredBuffer<uint16_t> myIntersectionMaskBuffer_BitCompressed : register(t0);

class CompressedIntersectionBits
{
	uint16_t myCompressedData;

	static CompressedIntersectionBits read_const(StructuredBuffer<uint16_t> aIntersectionMaskBuffer_BitCompressed_in, const uint aRayIndex_in)
	{
		CompressedIntersectionBits aCompressedIntersectionBits;
		aCompressedIntersectionBits.myCompressedData = aIntersectionMaskBuffer_BitCompressed_in[aRayIndex_in];
		return aCompressedIntersectionBits;
	}
};

class CompressedIntersectionBits_Test
{
	static CompressedIntersectionBits read_const(StructuredBuffer<uint16_t> aIntersectionMaskBuffer_BitCompressed_in, const uint aRayIndex_in)
	{
		CompressedIntersectionBits aCompressedIntersectionBits;
		aCompressedIntersectionBits.myCompressedData = aIntersectionMaskBuffer_BitCompressed_in[aRayIndex_in];
		return aCompressedIntersectionBits;
	}
};

void runKernel(const uint aIndexX_in, const uint aIndexY_in)
{
	uint aRayIndex = aIndexY_in * 512 + aIndexX_in;

	CompressedIntersectionBits aCompressedIntersectionBits = CompressedIntersectionBits::read_const(myIntersectionMaskBuffer_BitCompressed, aRayIndex);
	//CompressedIntersectionBits aCompressedIntersectionBits = CompressedIntersectionBits_Test::read_const(myIntersectionMaskBuffer_BitCompressed, aRayIndex);
}

[numthreads(8, 8, 1)]
void main(uint3 GlobalInvocationID : SV_DispatchThreadID, uint3 LocalInvocationID : SV_GroupThreadID)
{
	runKernel(GlobalInvocationID[0], GlobalInvocationID[1]);
}

Command Line used:
dxc.exe -spirv -T cs_6_7 -fvk-use-scalar-layout -enable-16bit-types -fspv-target-env=vulkan1.3 -fspv-debug=vulkan-with-source -E main

dxc version used:
dxcompiler.dll: 1.7 - 1.7.0.3774 (2168dcb4f)

Operating System used:
Windows 10

When removing the option
-fspv-debug=vulkan-with-source
the file compiles without a problem.

The issue also vanishes, when calling

CompressedIntersectionBits_Test::read_const(myIntersectionMaskBuffer_BitCompressed, aRayIndex);

instead of

CompressedIntersectionBits::read_const(myIntersectionMaskBuffer_BitCompressed, aRayIndex);
@Keenuts Keenuts added the bug Bug, regression, crash label Jan 5, 2023
@Keenuts Keenuts changed the title Shader failes to compile when activating debug infos [SPIR-V] Shader failes to compile when activating debug infos Jan 5, 2023
@Keenuts
Copy link
Collaborator

Keenuts commented Jan 5, 2023

Hi! Thanks for complete the report!

Was able to reproduce at head with:

class MyClass
{
	static MyClass my_func()
	{
		MyClass tmp;
		return tmp;
	}
};

[numthreads(1, 1, 1)]
void main(uint3 GlobalInvocationID : SV_DispatchThreadID)
{
  const uint index = GlobalInvocationID[0];
	MyClass::my_func();
}
./build/bin/dxc -spirv -T cs_6_7 -fspv-debug=vulkan-with-source -E main repro.hlsl

The issue seems to be linked to the optimizer, and the compact-ids pass, IDs gets rearranged, and the ID ends up being used before being defined:

10747 %26 = OpExtInst %void %1 DebugLexicalBlock %12 %uint_4 %uint_2 %22
10748 %28 = OpExtInst %void %1 DebugLocalVariable %27 %19 %12 %uint_5 %uint_11 %26 %uint_4
10853 %24 = OpExtInst %void %1 DebugTypeFunction %uint_3 %26
[...]
10855 %26 = OpExtInst %void %1 DebugTypeComposite %8 %uint_0 %19 %uint_1 %uint_7 %20 %8 %uint_0 %uint_3 %27

@Keenuts
Copy link
Collaborator

Keenuts commented Jan 5, 2023

Not a compact-ids pass issue, the initial code also contains other debug instructions with IDs defined later on. Also, from other generated spv, the use-before-declaration seems to be OK. Looking into it.

@Keenuts Keenuts self-assigned this Jan 5, 2023
@pow2clk pow2clk added the spirv Work related to SPIR-V label Jan 17, 2023
@webez
Copy link

webez commented Mar 6, 2023

Hi.
I could reproduce the same bug with this code

class ClassA {
  void FuncA()   {
      this.FuncB();
  }
  
    void FuncB()   {
  }
};

void main() {
    ClassA var;
    var.FuncA();
}

@Keenuts
Copy link
Collaborator

Keenuts commented May 22, 2023

Hello! Sorry haven't had much time for DXC.
Seems like this issue is related to KhronosGroup/SPIRV-Tools#5229 but this is not enough to fix it.
Seems like there is also a fix to do in the debug instruction ordering code.

Keenuts added a commit to Keenuts/DirectXShaderCompiler that referenced this issue May 23, 2023
The blanket rule for NonSemantic.Shader.DebugInfo.100 is "no forward
references". But there is a type-specific rule: DebugTypeComposite
members can be forward references:

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

This is mandatory to correctly support HLSL class methods:
 - the composite has a function as member.
 - the function as the composite as scope.
This cycle makes the forward reference mandatory, hence the exception
in the spec.

The removed code wrongly implemented the general rule, ignoring this
exception.

This change doesn't just solves the cycle issue, but might add forward
references that were avoided (since we never visit members). But as far
as the spec goes, this is a valid behavior. So I believe there is no
need to add any additional logic to avoid forward references.

This PR depends on the validator to correctly validate those forward
references (KhronosGroup/SPIRV-Tools#5230).

Fixes: microsoft#4911.

Signed-off-by: Nathan Gauër <[email protected]>
@Clothoid1
Copy link
Author

Clothoid1 commented Jul 18, 2023

I just tested this issue with latest DXC compiler.
I can now no longer reproduce this issue.
Instead i only get the warning:
warning : Member functions will not be linked to their class in the debug information. See KhronosGroup/SPIRV-Registry#203

Theoretically this issue could now be closed.
Or should it be kept open until also the warning is solved?

@Clothoid1
Copy link
Author

Clothoid1 commented Jul 18, 2023

When activating debug infos now all of our shaders compile - with the exception of our raytracing shaders.
But it seems to me that this remaining issue was already reported:
#5113

@Keenuts
Copy link
Collaborator

Keenuts commented Jul 18, 2023

Hello! So yes, thanks, for now, the issue is "resolved" as "it doesn't crash".
We can close this, but we now need to change the SPIR-V spec to solve this long-term.
(This will take longer, as we need to define the spec, and figure out how to propagate it, maybe wait a spirv release?)

For the second issue, seems unrelated to this (as in it's a different error/crash).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, regression, crash spirv Work related to SPIR-V
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants