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

Fix issue in peepholeOptimize #6543

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kaizhangNV
Copy link
Contributor

Close #6541.

When calculating the field index during peepholeOptimize, we should not count the field which is void type, because we don't use that field when calling makeStruct.

@kaizhangNV kaizhangNV requested a review from a team as a code owner March 7, 2025 23:12
@kaizhangNV kaizhangNV changed the title Fix issue in peepholeOptimize WIP: Fix issue in peepholeOptimize Mar 7, 2025
@kaizhangNV kaizhangNV added the pr: non-breaking PRs without breaking changes label Mar 7, 2025
Close shader-slang#6541.

When calculating the field index during peepholeOptimize, we should not
count the field which is void type, because we don't use that field when
calling makeStruct.
@@ -402,6 +402,10 @@ struct PeepholeContext : InstPassBase
Index i = 0;
for (auto sfield : structType->getFields())
{
if (as<IRVoidType>(sfield->getFieldType()) &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems the problem is not with peephole optimization, but the code that results the consistency between struct type and make struct. Whenever we remove the void field from the struct type, we also need to update all make struct insts to keep consistent.

@csyonghe
Copy link
Collaborator

csyonghe commented Mar 8, 2025

We need to find the pass that causes inconsistency between make struct and struct type, and make sure we never violate the invariant that make struct has the same number of operands as the number of struct fields.

@kaizhangNV kaizhangNV changed the title WIP: Fix issue in peepholeOptimize Fix issue in peepholeOptimize Mar 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: non-breaking PRs without breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid code emit for Metal
2 participants