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

Transfer offset decorations when legalizing laid-out structs #5525

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aleino-nv
Copy link
Collaborator

Struct legalization removing fields not representable in memory should transfer offset decorations in case the struct has already had offsets calculated.

Closes #5264.

Struct legalization removing fields not representable in memory should transfer offset
decorations in case the struct has already had offsets calculated.

Closes shader-slang#5264.
@aleino-nv aleino-nv requested a review from a team as a code owner November 8, 2024 11:32
@aleino-nv aleino-nv added the pr: non-breaking PRs without breaking changes label Nov 8, 2024
@aleino-nv
Copy link
Collaborator Author

/format

@slangbot
Copy link
Contributor

slangbot commented Nov 8, 2024

🌈 Formatted, please merge the changes from this PR

// In case the original struct had offsets calculated, transfer those as well.
// The original offsets should still be valid, since we only skip fields of types
// that aren't representable in memory.
originalField->transferChildrenOfTypeTo<IROffsetDecoration>(newField);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just transfer all decorations over.

@@ -780,8 +793,20 @@ struct IRInst
void removeOperand(Index index);

/// Transfer any decorations of this instruction to the `target` instruction.
// TODO: rewrite as transferDecorationsTo(target) ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Superfluous comment.

@@ -593,6 +593,19 @@ struct IRInst
IRDecoration* getLastDecoration();
IRInstList<IRDecoration> getDecorations();

// Step to the next child of a certain type, T
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don’t need this, just copy all decorations.

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.

WGSL: Nullpointer dereference in WGSLSourceEmitter::emitStructFieldAttributes
3 participants