Skip to content
Snippets Groups Projects

tests: Add missing require directives.

Closed Francisco Casas requested to merge fcasas/vkd3d:waveops-intel-fix into master
1 unresolved thread

There are 4 test files that have been failing for a while on my Intel UHD Graphics 770.

This is because these tests require support for typed UAV load additional formats and double-precision. So I added the typed uav load additional formats 'require' directive and the missing float64 require directives.

Edited by Francisco Casas

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Francisco Casas changed the description

    changed the description

    • diff --git a/tests/hlsl/sm6-uav-rwtexture.shader_test b/tests/hlsl/sm6-uav-rwtexture.shader_test
      index 1e4cc639d..d2fa1f04a 100644
      --- a/tests/hlsl/sm6-uav-rwtexture.shader_test
      +++ b/tests/hlsl/sm6-uav-rwtexture.shader_test
      @@ -1,5 +1,6 @@
       [require]
       shader model >= 6.0
      +typed uav load additional formats
       
       [uav 0]
       format r32 float

      I would prefer a more succinct string for this, although I don't necessarily have a good suggestion. Also, I think this particular test only uses R32_FLOAT, which should always be supported? We probably want to print whether the cap is available in run_shader_tests(), like we do for e.g. "float64" and "int64", although I notice we're missing "wave ops".

      One alternative I see would be to declare required format features in the require section. E.g.:

      [require]
      shader model >= 4.0
      r32g32b32a32-float uav-typed-load msaa-4
      b4g4r4a4-unorm texture2d uav-atomic-add
      ...

      It would be a bit more work to integrate that, but ultimately it seems like it would be able to cover more of our use-cases. How do you feel about something like that?

    • Agreed, it's probably clearer to query for individual formats and capabilities rather than use generic flags. This also makes the test able to run on more configurations, especially on exotic ones where the general flag cannot be enabled for some reason, but which are good enough to run the test anyway.

      I'd suggest to prepend the format capabilities lines with some keyword though. It makes parsing easier both for humans and machines, I think. Something like:

      format <format> <cap1> <cap2> ...

      And I agree that the problem with sm6-uav-rwtexture.shader_test looks different. It uses a format which should always be supported, and it fails in a way that looks different from the other two tests. My failure (on an Intel(R) UHD Graphics 630 (CFL GT2)) is:

      shader_runner:1093: sm6-uav-rwtexture.shader_test:27: Test failed: Got {1.00000000e+00, 0.00000000e+00, 3.00000000e+00, 4.00000000e+00}, expected {1.00000000e+00, 2.00000000e+00, 3.00000000e+00, 4.00000000e+00} at (0, 0).

      This looks more like a driver bug when addressing UAVs with unusual write masks rather than a missing format capability. I haven't investigated in depth, though.

    • Author Developer

      I would prefer a more succinct string for this, although I don't necessarily have a good suggestion.

      "uav_formats" seems good to me. But I would prefer to keep using caps->typed_uav_load_additional_formats internally so that it is easier to remember that it maps to D3D12_FEATURE_DATA_D3D12_OPTIONS.TypedUAVLoadAdditionalFormats.

      Also, I think this particular test only uses R32_FLOAT, which should always be supported?

      [uav 1] is R32G32B32A32_FLOAT because that is the default when there is no format directive. Also, when compiling the first test with DXC, the compiler points out that "Typed UAV Load Additional Formats" is required by the shader.

      Like Gio, If I run the test with the intel gpu it fails returning (1, 0, 3, 4) instead of (1, 2, 3, 4).

      We probably want to print whether the cap is available in run_shader_tests(), like we do for e.g. "float64" and "int64", although I notice we're missing "wave ops".

      Done.


      Regarding specifying the required format features in the require section, my initial inclination would be to avoid the additional complexity, since MSDN[1] explicitly says:

      The following formats are supported as a set on D3D12 or D3D11.3 hardware, so if any one is supported, all are supported.

      for all the formats that the TypedUAVLoadAdditionalFormats field covers. The support for other formats is checked individually.

      But I acknowledge that my experience with device requirements is thin. I will keep taking a look at this.


      [1] https://learn.microsoft.com/en-us/windows/win32/direct3d12/typed-unordered-access-view-loads

    • "uav_formats" seems good to me. But I would prefer to keep using caps->typed_uav_load_additional_formats internally so that it is easier to remember that it maps to D3D12_FEATURE_DATA_D3D12_OPTIONS.TypedUAVLoadAdditionalFormats.

      In that case I would suggest uav formats, to keep consistent with the style of other similar flags (which I don't love, but that's another matter). Maybe even additional uav formats, since a few formats are supported anyway, AFAIU.

      [uav 1] is R32G32B32A32_FLOAT because that is the default when there is no format directive. Also, when compiling the first test with DXC, the compiler points out that "Typed UAV Load Additional Formats" is required by the shader.

      Hmm, if the default format requires a specific flag to work then maybe it might be smarter to choose another default. Could you look into changing that?

      BTW, it's not a problem with this MR, but I am a bit confused about how TypedUAVLoadAdditionalFormats is implemented in vkd3d: it seems that it is set when shaderStorageImageReadWithoutFormat is enabled (and the various formats are available), but I don't understand why. Even when the SM4 shader declares it needs typed UAV load additional formats, the SPIR-V shader generated by vkd3d-shader doesn't seem to require reading without format. Why would it need it? What's the rationale here?

    • Please register or sign in to reply
  • Francisco Casas added 2 commits

    added 2 commits

    • ab23b4b2 - tests/shader-runner: Require typed UAV load additional formats.
    • 52bfda44 - tests: Add missing double precission require directives.

    Compare with previous version

  • Regarding specifying the required format features in the require section, my initial inclination would be to avoid the additional complexity, since MSDN[1] explicitly says:

    The following formats are supported as a set on D3D12 or D3D11.3 hardware, so if any one is supported, all are supported.

    for all the formats that the TypedUAVLoadAdditionalFormats field covers. The support for other formats is checked individually.

    Well, it's a bit of a trade-off. Sure, just checking "TypedUAVLoadAdditionalFormats" is easier for the d3d11 and d3d12 runners, but it's a bit of a pain for the Vulkan and GL runners. Perhaps conveniently, the MR currently only adds support for this feature to the d3d12 runner, but the Vulkan and GL runners are certainly capable of running these tests.

  • Francisco Casas mentioned in merge request !987 (merged)

    mentioned in merge request !987 (merged)

  • Francisco Casas mentioned in merge request !991 (merged)

    mentioned in merge request !991 (merged)

  • Author Developer

    Closing, superseded by !991 (merged).

Please register or sign in to reply
Loading