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

Per-function feature flags and min target in RDAT are incorrect #6218

Closed
tex3d opened this issue Jan 29, 2024 · 0 comments · Fixed by #6207
Closed

Per-function feature flags and min target in RDAT are incorrect #6218

tex3d opened this issue Jan 29, 2024 · 0 comments · Fixed by #6207
Assignees
Labels
bug Bug, regression, crash sm6.8 Shader Model 6.8

Comments

@tex3d
Copy link
Contributor

tex3d commented Jan 29, 2024

Description
RDAT has per-function feature requirement flags, compatible shader stage flags, and a minimum shader target that should take into account internal function calls and should adjust the minimum shader targets and legal shader stages based on optional feature usage.

Several issues were preventing these from correctly representing the requirements for the function:

  • per-function DXIL Shader Flag collection has bugs:
    • missing some feature usage scenarios
    • using global resources to indicate flags that may not be applicable to the function.
    • can't account for stage-specific flags (DerivativesInMeshAndAmplificationShaders)
  • Features used in internally called functions were not accounted for in the caller.
  • Min shader model wasn't adjusted by any feature usage flags introduced since SM 6.2.
  • Min shader model didn't take into account newer shader stages (ms/as introduced in SM 6.5).
  • Min shader model couldn't account for stage-specific feature availability differences (derivatives used in CS, MS, or AS).
  • WaveSize and WaveSizeRange fail to impact minimum shader target.

This also impacts module-level flags, since they are just the union of flags computed for each function, plus a few module-scope checks.

Fixing these issues can cause differences in the DFCC_RuntimeData (RDAT) part as well as DxilModule ShaderFlags (in metadata) and the DFCC_FeatureInfo (SFI0) parts. This would be considered a validator-breaking change, since the validator uses the same code to compute these details, then makes sure the container matches these expectations.

Steps to Reproduce
The following reflection test demonstrates some of these bugs based on the difference between expected result from 1.7 and fixed 1.8 validator:

// RUN: %dxilver 1.8 | %dxc -T lib_6_8 %s | %D3DReflect %s | %FileCheck %s -check-prefixes=RDAT,RDAT18
// RUN: %dxilver 1.7 | %dxc -T lib_6_7 -validator-version 1.7 %s | %D3DReflect %s | FileCheck %s -check-prefixes=RDAT,RDAT17

// Ensure min shader target incorporates optional features used

// RDAT: FunctionTable[{{.*}}] = {

// SM 6.7+

///////////////////////////////////////////////////////////////////////////////
// ShaderFeatureInfo_WriteableMSAATextures (0x40000000) = 1073741824

// RDAT-LABEL: UnmangledName: "rwmsaa"
// ShaderFeatureInfo_WriteableMSAATextures (0x40000000) = 1073741824
// RDAT:   FeatureInfo1: 1073741824
// RDAT:   FeatureInfo2: 0
// MinShaderTarget: (Library(6) << 16) + (SM 6.7 ((6 << 4) + 7)) = 0x60067 = 393319
// RDAT18: MinShaderTarget: 393319
// Old: 6.0
// RDAT17: MinShaderTarget: 393312

RWByteAddressBuffer BAB : register(u1, space0);
RWTexture2DMS<float, 1> T2DMS : register(u2, space0);

[noinline] export
void rwmsaa() {
  // Use dynamic resource to avoid MSAA flag on all functions issue in 1.7
  // RWTexture2DMS<float, 1> T2DMS = ResourceDescriptorHeap[0];
  BAB.Store(0, T2DMS.sample[1][uint2(1, 2)]);
}

// RDAT-LABEL: UnmangledName: "no_rwmsaa"
// RDAT18: FeatureInfo1: 0
// 1.7 Incorrectly reports feature use for function
// RDAT17: FeatureInfo1: 1073741824
// RDAT:   FeatureInfo2: 0
// MinShaderTarget: (Library(6) << 16) + (SM 6.0 ((6 << 4) + 0)) = 0x60060 = 393312
// RDAT: MinShaderTarget: 393312

export void no_rwmsaa() {
  BAB.Store(0, 0);
}

// RDAT-LABEL: UnmangledName: "rwmsaa_in_raygen"
// ShaderFeatureInfo_WriteableMSAATextures (0x40000000) = 1073741824
// Old: set because of global
// RDAT: FeatureInfo1: 1073741824
// RDAT:   FeatureInfo2: 0
// MinShaderTarget: (RayGeneration(7) << 16) + (SM 6.7 ((6 << 4) + 7)) = 0x70067 = 458855
// RDAT18: MinShaderTarget: 458855
// Old: 6.0
// RDAT17: MinShaderTarget: 458848

[shader("raygeneration")]
void rwmsaa_in_raygen() {
  rwmsaa();
}

// RDAT-LABEL: UnmangledName: "rwmsaa_not_used_in_raygen"
// ShaderFeatureInfo_WriteableMSAATextures (0x40000000) = 1073741824
// RDAT18: FeatureInfo1: 0
// Old: set because of global
// RDAT17: FeatureInfo1: 1073741824
// RDAT:   FeatureInfo2: 0
// MinShaderTarget: (RayGeneration(7) << 16) + (SM 6.3 ((6 << 4) + 3)) = 0x70063 = 458851
// RDAT18: MinShaderTarget: 458851
// Old: 6.0
// RDAT17: MinShaderTarget: 458848

[shader("raygeneration")]
void rwmsaa_not_used_in_raygen() {
  no_rwmsaa();
}

Expected Behavior
Optional feature use and potential optional feature use is always captured into flags. Min shader model is correct when combining all called functions and any special-case logic required for a final entry point function of a particular kind.
For the example above, when expected is different than actual, expected values are shown in the RDAT18 checks.

Actual Behavior
See above description of cases not captured correctly, as well as the RDAT17 sections of the example test case above.

Environment

  • DXC version: recent main branch (dcb618a2f1b7f4265fc2b77c12291f76eb334ae3).
  • Host Operating System: any
@tex3d tex3d added bug Bug, regression, crash needs-triage Awaiting triage labels Jan 29, 2024
@tex3d tex3d self-assigned this Jan 29, 2024
@tex3d tex3d removed this from HLSL Triage Jan 29, 2024
@github-project-automation github-project-automation bot moved this to 🆕 New in HLSL Roadmap Jan 29, 2024
@tex3d tex3d removed the needs-triage Awaiting triage label Jan 29, 2024
@tex3d tex3d added this to the Shader Model 6.8 milestone Jan 29, 2024
@pow2clk pow2clk moved this from 🆕 New to 🏗 In progress in HLSL Roadmap Jan 29, 2024
tex3d added a commit that referenced this issue Feb 10, 2024
…ging (#6207)

Add ShaderKind::Last_1_8 for shader mask
Add shader model comments before flag groupings in DxilConstants.h and DxilShaderFlags.

Add missing flag checks for min shader model in RDAT function info. Move ShaderCompatInfo computation into DxilModule, propagate callee info.

Move computation of shader model requirements based on flags into DxilModule. Finalize requirements for entry functions in AdjustMinimumShaderModelAndFlags.

Fixes for function level flag tracking:
- DerivativesInMeshAndAmpShaders: use flag to track deriv use, then adjust for entry functions.
- hasUAVs: based on resource use in function instead of global resources.
- WriteableMSAATextures: based on use in function instead of global resources.
  - Also catch cases for dynamic res from any use by looking at create/annotate handle, not just the TextureStoreSample op.
- RaytracingTier1_1: move module-level detection to CollectShaderFlagsForModule
- Marked deriv and quad ops as being supported in node.
- Fixed SampleCmpBias to be considered gradient op.
- Update RDAT definitions to dump more useful info for testing

Fixes #6218.
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in HLSL Roadmap Feb 10, 2024
tex3d added a commit to tex3d/DirectXShaderCompiler that referenced this issue Feb 10, 2024
…ging (microsoft#6207)

Add ShaderKind::Last_1_8 for shader mask
Add shader model comments before flag groupings in DxilConstants.h and DxilShaderFlags.

Add missing flag checks for min shader model in RDAT function info. Move ShaderCompatInfo computation into DxilModule, propagate callee info.

Move computation of shader model requirements based on flags into DxilModule. Finalize requirements for entry functions in AdjustMinimumShaderModelAndFlags.

Fixes for function level flag tracking:
- DerivativesInMeshAndAmpShaders: use flag to track deriv use, then adjust for entry functions.
- hasUAVs: based on resource use in function instead of global resources.
- WriteableMSAATextures: based on use in function instead of global resources.
  - Also catch cases for dynamic res from any use by looking at create/annotate handle, not just the TextureStoreSample op.
- RaytracingTier1_1: move module-level detection to CollectShaderFlagsForModule
- Marked deriv and quad ops as being supported in node.
- Fixed SampleCmpBias to be considered gradient op.
- Update RDAT definitions to dump more useful info for testing

Fixes microsoft#6218.

(cherry picked from commit 9c518db)
tex3d added a commit that referenced this issue Feb 10, 2024
…ging (#6207) (#6273)

Add ShaderKind::Last_1_8 for shader mask
Add shader model comments before flag groupings in DxilConstants.h and
DxilShaderFlags.

Add missing flag checks for min shader model in RDAT function info. Move
ShaderCompatInfo computation into DxilModule, propagate callee info.

Move computation of shader model requirements based on flags into
DxilModule. Finalize requirements for entry functions in
AdjustMinimumShaderModelAndFlags.

Fixes for function level flag tracking:
- DerivativesInMeshAndAmpShaders: use flag to track deriv use, then
adjust for entry functions.
- hasUAVs: based on resource use in function instead of global
resources.
- WriteableMSAATextures: based on use in function instead of global
resources.
- Also catch cases for dynamic res from any use by looking at
create/annotate handle, not just the TextureStoreSample op.
- RaytracingTier1_1: move module-level detection to
CollectShaderFlagsForModule
- Marked deriv and quad ops as being supported in node.
- Fixed SampleCmpBias to be considered gradient op.
- Update RDAT definitions to dump more useful info for testing

Fixes #6218.

(cherry picked from commit 9c518db)
@pow2clk pow2clk added the sm6.8 Shader Model 6.8 label Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, regression, crash sm6.8 Shader Model 6.8
Projects
Archived in project
3 participants
@tex3d @pow2clk and others