Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

[spirv] fix DebugTypeArray bug#312

Merged
jaebaek merged 4 commits intogoogle:debug_infofrom
jaebaek:dbg_type_array
Jun 5, 2020
Merged

[spirv] fix DebugTypeArray bug#312
jaebaek merged 4 commits intogoogle:debug_infofrom
jaebaek:dbg_type_array

Conversation

@jaebaek
Copy link
Copy Markdown

@jaebaek jaebaek commented May 29, 2020

The pseudo code for lowering an array type in DebugTypeVisitor is

elementType = arrayType->element
if elementType is an array type
  (DebugTypeArray for elementType).appendOneMoreDimensionAtTheEnd(count)
else
  new DebugTypeArray(lower(arrayType->element), count)

When it appends one more dimension at the end of previously created
DebugTypeArray, it reuses the existing DebugTypeArray object, which
result in a wrong DebugTypeArray for a previously shown array type.

For example,

float foo[2];  --> dbg_type_array = DebugTypeArray(float, 2)
float bar[2][3]; --> dbg_type_array.appendOneMoreDimensionAtTheEnd(3)

// When emitting the SPIRV, dbg_type_array is
// DebugTypeArray(float, 2, 3) which is incorrect for "foo"

This commit lets DebugTypeVisitor create a new DebugTypeArray object for
new array type to fix this bug.

@jaebaek jaebaek self-assigned this May 29, 2020
@jaebaek
Copy link
Copy Markdown
Author

jaebaek commented May 29, 2020

Ben found this bug by compiling the Sascha-William skeletal animation shader.

@AppVeyorBot
Copy link
Copy Markdown

The pseudo code for lowering an array type in DebugTypeVisitor is
```
elementType = arrayType->element
if elementType is an array type
  (DebugTypeArray for elementType).appendOneMoreDimensionAtTheEnd(count)
else
  new DebugTypeArray(lower(arrayType->element), count)
```

When it appends one more dimension at the end of previously created
DebugTypeArray, it reuses the existing DebugTypeArray object, which
result in a wrong DebugTypeArray for a previously shown array type.

For example,
```
float foo[2];  --> dbg_type_array = DebugTypeArray(float, 2)
float bar[2][3]; --> dbg_type_array.appendOneMoreDimensionAtTheEnd(3)

// When emitting the SPIRV, dbg_type_array is
// DebugTypeArray(float, 2, 3) which is incorrect for "foo"
```
This commit lets DebugTypeVisitor create a new DebugTypeArray object for
new array type to fix this bug.
@AppVeyorBot
Copy link
Copy Markdown

@AppVeyorBot
Copy link
Copy Markdown

Comment thread tools/clang/lib/SPIRV/DebugTypeVisitor.cpp Outdated
Comment thread tools/clang/lib/SPIRV/EmitVisitor.cpp Outdated
Copy link
Copy Markdown
Author

@jaebaek jaebaek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your code review. PTAL!

Comment thread tools/clang/lib/SPIRV/DebugTypeVisitor.cpp Outdated
Comment thread tools/clang/lib/SPIRV/EmitVisitor.cpp Outdated
@AppVeyorBot
Copy link
Copy Markdown

@jaebaek jaebaek merged commit 8333f13 into google:debug_info Jun 5, 2020
@jaebaek jaebaek deleted the dbg_type_array branch June 5, 2020 18:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants