Added implementation for ID3D12FunctionReflection1::GetDesc1

[Nielsbishere]

Relates to microsoft/DirectX-Headers#135.

I have tested this version in my own project and it seems to work. Only thing is that this requires the DirectX-Headers dep to be updated to ensure D3D12_FUNCTION_DESC1 and co can be found.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 9018970f265e4152a4313044afc06818aa635a2a e24d9888d62efe13028436da81b15100a32b9c4b -- include/dxc/Support/D3DReflection.h include/dxc/Support/Unicode.h include/dxc/Test/D3DReflectionDumper.h include/dxc/Test/D3DReflectionStrings.h include/dxc/WinAdapter.h lib/DxcSupport/Unicode.cpp lib/DxcSupport/dxcapi.use.cpp lib/DxilContainer/D3DReflectionDumper.cpp lib/DxilContainer/D3DReflectionStrings.cpp lib/HLSL/DxilContainerReflection.cpp projects/dxilconv/tools/dxbc2dxil/dxbc2dxil.cpp tools/clang/include/clang/AST/Attr.h tools/clang/tools/dxa/dxa.cpp tools/clang/tools/dxcvalidator/dxcvalidator.cpp tools/clang/tools/dxr/dxr.cpp tools/clang/unittests/HLSLExec/ExecutionTest.cpp tools/clang/unittests/HLSLTestLib/FileCheckerTest.cpp tools/dxexp/dxexp.cpp

View the diff from clang-format here.

diff --git a/lib/HLSL/DxilContainerReflection.cpp b/lib/HLSL/DxilContainerReflection.cpp
index d0573d829..08f150990 100644
--- a/lib/HLSL/DxilContainerReflection.cpp
+++ b/lib/HLSL/DxilContainerReflection.cpp
@@ -2876,7 +2876,7 @@ public:
   // Use D3D_RETURN_PARAMETER_INDEX to get description of the return value.
   STDMETHOD_(ID3D12FunctionParameterReflection *, GetFunctionParameter)
   (INT ParameterIndex) { return &g_InvalidFunctionParameter; }
-  
+
   STDMETHOD_(UINT64, GetRequiresFlags)() { return m_FeatureFlags; }
 };
 

• Check this box to apply formatting changes to this branch.

[damyanp]

Hi - thank you for all your effort here!

I just want to set appropriate expectations - this is a pretty large change adding a lot of new API surface area that we'll need to consider, taking into account future evolution of the language and compiler. As you may know we're hard at work adding HLSL support to clang, and so we need to consider the implications of a change that essentially exposes some amount of implementation details that may not be stable in the long term. Also, as we're focusing on clang, this means we have limited bandwidth for reviews of this nature, especially as we're in the middle of summer vacation season. So, be warned, it may take a while for us to find time to look at this properly.

In the meantime, I suggest you have a look at CONTRIBUTING.md to make sure that you have all your ducks in a row ready for a successful review. For example, I think some tests may be required.

Thanks again!

• Damyan.

[Nielsbishere]

Sounds reasonable, though I'm not sure if you're referring to unit tests or that the tests are failing on this PR.
Mainly the payloadSize/intersectionSize and some workgraph information is going to be useful for me. In my current parsers (also in production at work) I need to do custom work to parse struct sizes and other information that could hopefully be exposed even with a clang supported version of DXC. My goal there is to move as much as possible to a standardized DXC to avoid manual parser errors and missing functionality from my side. It is working in my PR, but it's tricky since this is dependent on the DirectX-Headers change. I updated the dependency, but still need to upstream some of the changes I had to make it build properly. If this PR is ever merged, it needs to change the external dependency's URL back from Oxsomi/DirectX-Headers to Microsoft/DirectX-Headers, since I have no other way to make the tests pass until that is merged.

[damyanp]

...I'm not sure if you're referring to unit tests...

I'm referring to unit tests with the code. From CONTRIBUTING.md:

All changes that make functional or behavioral changes to the compiler whether by fixing bugs or adding features must include additional testing in the implementing pull request.

[Nielsbishere]

Will take a look at Wednesday. Unexpectedly failing on remote. Also adding implementation for the reflection writer to know about these structs and output it as disassembly, along with a few unit tests.

[Nielsbishere]

This should now be ready for review. Unless support in the disassembler is required, though I'm not sure how useful it would be since dxa and the unit testing tool both provide this functionality through D3DReflectionDumper.

[Nielsbishere]

Added an important function to ID3D12FunctionReflection1; which is GetRequiresFlags such as in ID3D12ShaderReflection. Without this, it is impossible to find out what features are being used by a lib file (I thought the desc Flags was used for this, but nope).

This is especially important now that some GPUs don't always support the base featureset, so I use the following to check feature support of the underlying GPUs:

		D3D_SHADER_REQUIRES_RAYTRACING_TIER_1_1,
		D3D_SHADER_REQUIRES_NATIVE_16BIT_OPS,
		D3D_SHADER_REQUIRES_INT64_OPS,
		D3D_SHADER_REQUIRES_VIEW_ID,
		D3D_SHADER_REQUIRES_DOUBLES,
		D3D_SHADER_REQUIRES_11_1_DOUBLE_EXTENSIONS,
		D3D_SHADER_REQUIRES_ATOMIC_INT64_ON_TYPED_RESOURCE,
		D3D_SHADER_REQUIRES_ATOMIC_INT64_ON_GROUP_SHARED,
		D3D_SHADER_REQUIRES_DERIVATIVES_IN_MESH_AND_AMPLIFICATION_SHADERS,
		D3D_SHADER_REQUIRES_WRITEABLE_MSAA_TEXTURES,
		D3D_SHADER_REQUIRES_WAVE_OPS

I use this reflection to know whether a shader is really using a feature. It's possible someone enables I64 to include something that requires it but never actually uses I64 in the final shader. This is then used to retroactively disable that feature to ensure we don't go checking features that aren't really relevant.