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] DXC crashes when declaring a variable in multiple case block #6718

Closed
shobomaru opened this issue Jun 22, 2024 · 1 comment · Fixed by #6756
Closed

[SPIR-V] DXC crashes when declaring a variable in multiple case block #6718

shobomaru opened this issue Jun 22, 2024 · 1 comment · Fixed by #6756
Assignees
Labels
bug Bug, regression, crash spirv Work related to SPIR-V

Comments

@shobomaru
Copy link

Description
LLVM assertion failed.

Steps to Reproduce
dxc -T cs_6_0 -spirv path/to/file.hlsl

HLSL code:

uint Get(uint a)
{
        [branch]
        switch (a)
        {
        case 0: {
                uint d = a + 1;
                return a + 1;
        }
        case 1: {
                uint d1 = a + 2;
                return a + 2;
        }
        }
        return 0;
}

RWStructuredBuffer<uint> Out;
[numthreads(1, 1, 1)]
void main()
{
        uint a = Out[0];
        uint v = Get(a);
        Out[0] = v;
}

If change the code in one of the following ways, the issue is not occurred.

  • Remove [branch]
  • Remove uint d = a + 1; or uint d1 = a + 2; (in other words, only declaring one variable in the switch statement)

Actual Behavior

Assertion failed: (astDecls[var].instr == nullptr), function createFnVar, file DeclResultIdMapper.cpp, line 1106.

Environment

  • DXC version: libdxcompiler.dylib: 1.8(dev;4640-45018c75)
  • Host Operating System: Windows 11 23H2 and macOS 14.5
@shobomaru shobomaru added bug Bug, regression, crash needs-triage Awaiting triage spirv Work related to SPIR-V labels Jun 22, 2024
@Keenuts
Copy link
Collaborator

Keenuts commented Jul 9, 2024

Thanks for the issue!

Repro:

// RUN: %dxc -T cs_6_6 %s -E main -fcgl -spirv | FileCheck %s

[numthreads(1, 1, 1)]
void main()
{
  uint a = 0;

  [branch]
  switch (a) {
    default: return;
    case 1: {
      uint d1;
    }
  }
}

Looks like this is caused by our code to map variable to declaration: we process 2 VarDecl nodes that declare the same variable. And there is an assert for that.
The base AST is fine:

  |   `-SwitchStmt 0x55fe2c490258 <line:9:3, line:14:3>
  |     |-<<<NULL>>>
  |     |-ImplicitCastExpr 0x55fe2c490240 <line:9:11> 'uint':'unsigned int' <LValueToRValue>
  |     | `-DeclRefExpr 0x55fe2c490200 <col:11> 'uint':'unsigned int' lvalue Var 0x55fe2c490140 'a' 'uint':'unsigned int'
  |     `-CompoundStmt 0x55fe2c4903e0 <col:14, line:14:3>
  |       |-DefaultStmt 0x55fe2c490298 <line:10:5, col:14>
  |       | `-ReturnStmt 0x55fe2c490280 <col:14>
  |       `-CaseStmt 0x55fe2c4902d8 <line:11:5, line:13:5>
  |         |-ImplicitCastExpr 0x55fe2c490408 <line:11:10> 'uint':'unsigned int' <IntegralCast>
  |         | `-IntegerLiteral 0x55fe2c4902b8 <col:10> 'literal int' 1
  |         |-<<<NULL>>>
  |         `-CompoundStmt 0x55fe2c4903a8 <col:13, line:13:5>
  |           `-DeclStmt 0x55fe2c490390 <line:12:7, col:14>
  |             `-VarDecl 0x55fe2c490320 <col:7, col:12> col:12 d1 'uint':'unsigned int'

But because of the [Branch] tag, we have the following AST we flatten the Switch, and seems like we do something wrong there.

IfStmt 0x555555a65920
|-<<<NULL>>>
|-BinaryOperator 0x555555a65958 'bool' '=='
| |-ImplicitCastExpr 0x555555a656a0 'uint':'unsigned int' <LValueToRValue>
| | `-DeclRefExpr 0x555555a65660 'uint':'unsigned int' lvalue Var 0x555555a655a0 'a' 'uint':'unsigned int'
| `-ImplicitCastExpr 0x555555a65868 'uint':'unsigned int' <IntegralCast>
|   `-IntegerLiteral 0x555555a65718 'literal int' 1
|-CompoundStmt 0x555555a65900
| `-DeclStmt 0x555555a657f0
|   `-VarDecl 0x555555a65780  d1 'uint':'unsigned int'
`-CompoundStmt 0x555555a658d8
  |-ReturnStmt 0x555555a656e0
  `-DeclStmt 0x555555a657f0
    `-VarDecl 0x555555a65780  d1 'uint':'unsigned int'

@Keenuts Keenuts self-assigned this Jul 9, 2024
@Keenuts Keenuts removed the needs-triage Awaiting triage label Jul 9, 2024
Keenuts added a commit to Keenuts/DirectXShaderCompiler that referenced this issue Jul 9, 2024
When the [branch] annotation is used, switches are converted into
an if/else tree.
Issue arise when declaring a variable into a switch case:
 - when flattened, the same variable could be traversed twice in case
   of a case fall-through.

In addition, there was a small bug in the flattening logic: only breaks
were stopping the handling of further statements, while early-returns
were ignored. This was not an important bug as it only added more
dead-code, but it was wrong.

Fixes microsoft#6718

Signed-off-by: Nathan Gauër <brioche@google.com>
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
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants