Skip to content

Intrinsics ddx_fine/ddy_fine should not be allowed to sink into flow control #5744

New issue

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

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

Already on GitHub? # to your account

Open
hekota opened this issue Sep 18, 2023 · 2 comments
Open
Labels
bug Bug, regression, crash correctness Bugs that impact shader correctness
Milestone

Comments

@hekota
Copy link
Member

hekota commented Sep 18, 2023

Description
Intrinsics ddx_fine/ddy_fine should not be allowed to sink into flow control. That can currently happen because they are marked ReadNone same as other unary ops, but they are reading values from neighboring threads in the same quad.

Steps to Reproduce

  1. Open tools/clang/unittests/HLSLExec/ShaderOpArith.xml, find data for HelperLaneTestNoWave and remove -opt-disable sink from the shader compile arguments.
  2. Run hcttest exec-filter *HelperLaneTest -verbose -adapter NVidia*

Actual Behavior

StartGroup: ExecutionTest::HelperLaneTest
Verifying IsHelperLane in shader model 6.0
Using Adapter:NVIDIA GeForce GTX 1660 Ti
Error: Verify: AreEqual(pTestResults[0].is_helper_00, 0) - Values (1, 0) [File: D:\dxc2\tools\clang\unittests\HLSLExec\ExecutionTest.cpp, Function: ExecutionTest::HelperLaneTest, Line: 10939]
EndGroup: ExecutionTest::HelperLaneTest [Failed]

Expected Behavior

StartGroup: ExecutionTest::HelperLaneTest
Verifying IsHelperLane in shader model 6.0
Using Adapter:NVIDIA GeForce GTX 1660 Ti
Verifying IsHelperLane in shader model 6.6
Using Adapter:NVIDIA GeForce GTX 1660 Ti
EndGroup: ExecutionTest::HelperLaneTest [Passed]

More info

This shader code

            int is_helper_accross_X = ReadAcrossX_DD(is_helper, isLeft);
            int is_helper_accross_Y = ReadAcrossY_DD(is_helper, isTop);
            int is_helper_accross_Diag = ReadAcrossDiagonal_DD(is_helper, isLeft, isTop);

            if (!isLeft && !isTop) { //bottom right pixel writes results
              g_testResults[i].is_helper_00 = is_helper_accross_Diag;
              g_testResults[i].is_helper_10 = is_helper_accross_Y;
              g_testResults[i].is_helper_01 = is_helper_accross_X;
              g_testResults[i].is_helper_11 = is_helper;
            }

is optimized into code equivalent to

            int is_helper_accross_X = ReadAcrossX_DD(is_helper, isLeft);

            if (!isLeft && !isTop) { //bottom right pixel writes results
              int is_helper_accross_Y = ReadAcrossY_DD(is_helper, isTop);
              int is_helper_accross_Diag = ReadAcrossDiagonal_DD(is_helper, isLeft, isTop);
              g_testResults[i].is_helper_00 = is_helper_accross_Diag;
              g_testResults[i].is_helper_10 = is_helper_accross_Y;
              g_testResults[i].is_helper_01 = is_helper_accross_X;
              g_testResults[i].is_helper_11 = is_helper;
            }

The ReadAcrossDiagonal_DD function is reading a value from neighboring thread in the same quad. That value has not been calculated because this call and ReadAcrossY_DD got moved under the if flow control and did not execute on all threads in the quad.

Environment

  • DXC version 634c2f3 or later (main branch from July 28 forward, after HLSL 2021 was enabled by default).
  • Windows
@hekota hekota added bug Bug, regression, crash needs-triage Awaiting triage labels Sep 18, 2023
@hekota hekota changed the title Intrinsics ddx_fine/ddy_fine intrinsics should not be allowed to sink into flow control Intrinsics ddx_fine/ddy_fine should not be allowed to sink into flow control Sep 18, 2023
hekota added a commit to hekota/DirectXShaderCompiler that referenced this issue Sep 18, 2023
Add `-opt-disable sink` to HelperLaneTest shader compilation to prevent sinking of `ddx_fine`/`ddy_fine` intrinsics into flow control.

Separate issue has been filed to track down the root of the problem:
microsoft#5744: Intrinsics ddx_fine/ddy_fine should not be allowed to sink into flow control
@github-project-automation github-project-automation bot moved this to 🆕 New in HLSL Roadmap Sep 19, 2023
@llvm-beanz llvm-beanz added the correctness Bugs that impact shader correctness label Sep 19, 2023
@pow2clk pow2clk removed the needs-triage Awaiting triage label Sep 19, 2023
@pow2clk pow2clk moved this to Done in HLSL Triage Sep 19, 2023
hekota added a commit that referenced this issue Sep 20, 2023
Add `-opt-disable sink` to HelperLaneTest shader compilation to prevent
sinking of `ddx_fine`/`ddy_fine` intrinsics into flow control.

Separate issue has been filed to track down the root of the problem:
#5744: Intrinsics ddx_fine/ddy_fine should not be allowed to sink into
flow control
llvm-beanz pushed a commit to llvm-beanz/DirectXShaderCompiler that referenced this issue Sep 21, 2023
Add `-opt-disable sink` to HelperLaneTest shader compilation to prevent
sinking of `ddx_fine`/`ddy_fine` intrinsics into flow control.

Separate issue has been filed to track down the root of the problem:
microsoft#5744: Intrinsics ddx_fine/ddy_fine should not be allowed to sink into
flow control
hekota added a commit to hekota/DirectXShaderCompiler that referenced this issue Sep 21, 2023
Add `-opt-disable sink` to HelperLaneTest shader compilation to prevent
sinking of `ddx_fine`/`ddy_fine` intrinsics into flow control.

Separate issue has been filed to track down the root of the problem:
microsoft#5744: Intrinsics ddx_fine/ddy_fine should not be allowed to sink into
flow control
hekota added a commit that referenced this issue Sep 22, 2023
Add `-opt-disable sink` to HelperLaneTest shader compilation to prevent
sinking of `ddx_fine`/`ddy_fine` intrinsics into flow control.

Separate issue has been filed to track down the root of the problem:
#5744: Intrinsics ddx_fine/ddy_fine should not be allowed to sink into
flow control
@simondeschenes
Copy link

I am hitting this problem, is there any known workaround?

@hekota
Copy link
Member Author

hekota commented Jan 8, 2024

The compile flag -opt-disable sink should help with that.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Bug, regression, crash correctness Bugs that impact shader correctness
Projects
Status: New
Status: Triaged
Development

No branches or pull requests

5 participants