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

Add alignment to NodeRecordType including DXIL metadata update #6279

Merged
merged 14 commits into from
Feb 29, 2024

Conversation

tex3d
Copy link
Contributor

@tex3d tex3d commented Feb 10, 2024

This change adds NodeRecordType alignment field to RDAT to make it possible to validate pointer and stride alignment in the runtime.

This includes a change to DXIL metadata to preserve the record alignment without requiring recovery by looking for GetNodeRecordPtr.

Fixes #6270

Copy link
Contributor

github-actions bot commented Feb 10, 2024

✅ With the latest revision this PR passed the Python code formatter.

@tex3d tex3d force-pushed the rdat-record-alignment branch 4 times, most recently from 800e5cf to eef8c29 Compare February 10, 2024 23:51
@tex3d tex3d force-pushed the rdat-record-alignment branch from eef8c29 to e0f9207 Compare February 20, 2024 21:33
@tex3d tex3d force-pushed the rdat-record-alignment branch from e0f9207 to c175bff Compare February 27, 2024 10:15
Since DxilValidation for RDAT part expects exact binary comparison match, we need to restore any missing alignment from the RDAT before we generate RDAT and compare.

Alignment for records used with GetNodeRecordPtr() will be restored from llvm, and not overwritten by this alignment from RDAT, so it still varifies that alignment matches type llvm module when available.
@tex3d tex3d force-pushed the rdat-record-alignment branch from c175bff to 43509fb Compare February 27, 2024 19:12
@tex3d tex3d marked this pull request as ready for review February 29, 2024 02:40
Originally, the test macro copied the record from input to output, but that was changed, so the name was stale.
@tex3d tex3d requested a review from pow2clk February 29, 2024 02:51
Copy link
Member

@hekota hekota left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@pow2clk pow2clk left a comment

Choose a reason for hiding this comment

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

Looks good!

@tex3d tex3d merged commit 66ba5a1 into microsoft:main Feb 29, 2024
13 checks passed
@tex3d tex3d deleted the rdat-record-alignment branch February 29, 2024 19:25
tex3d added a commit to tex3d/DirectXShaderCompiler that referenced this pull request Feb 29, 2024
…soft#6279)

This change adds NodeRecordType alignment field to RDAT to make it
possible to validate pointer and stride alignment in the runtime.

This includes a change to DXIL metadata to preserve the record alignment
without requiring recovery by looking for GetNodeRecordPtr.

Fixes microsoft#6270

(cherry picked from commit 66ba5a1)
tex3d added a commit that referenced this pull request Feb 29, 2024
#6374)

This change adds NodeRecordType alignment field to RDAT to make it
possible to validate pointer and stride alignment in the runtime.

This includes a change to DXIL metadata to preserve the record alignment
without requiring recovery by looking for GetNodeRecordPtr.

Fixes #6270

(cherry picked from commit 66ba5a1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

RDAT needs Node Record Alignment
3 participants