Skip to content

[DxbcConverter] Fix corruption of ICB integer values#4790

Merged
vettoreldaniele merged 1 commit intomicrosoft:mainfrom
ben-clayton:fix-4786
Nov 23, 2022
Merged

[DxbcConverter] Fix corruption of ICB integer values#4790
vettoreldaniele merged 1 commit intomicrosoft:mainfrom
ben-clayton:fix-4786

Conversation

@ben-clayton
Copy link
Copy Markdown
Contributor

Use an array of 32-bit integer values instead of 32-bit float values for the "dx.icb" immediate constant buffer data. This prevents the FPU flipping the 22nd bit of integer constants, when their bit pattern represents a float32 signalling NaN on x86 architecture.

Fixes #4786

Use an array of 32-bit integer values instead of 32-bit float values for the `"dx.icb"` immediate constant buffer data.
This prevents the FPU flipping the 22nd bit of integer constants, when their bit pattern represents a float32 signalling NaN on x86 architecture.

Fixes microsoft#4786
@ben-clayton
Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree company="Google"

@AppVeyorBot
Copy link
Copy Markdown

@pow2clk pow2clk requested a review from tex3d November 15, 2022 21:53
Copy link
Copy Markdown
Member

@jenatali jenatali left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@ben-clayton
Copy link
Copy Markdown
Contributor Author

Thank you for the review. I do not have merge permissions, so if you are happy with this landing, please will you do the honours?

Thanks,
Ben

@jenatali
Copy link
Copy Markdown
Member

I'd like to wait for someone on the actual shader compiler team to take a look. While I've got some experience in this area, it's not my place to merge this without their signoff.

Copy link
Copy Markdown
Contributor

@tex3d tex3d left a comment

Choose a reason for hiding this comment

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

This looks good to me too.

@ben-clayton
Copy link
Copy Markdown
Contributor Author

Thank you for the reviews. Please can someone with permissions land this change?

@vettoreldaniele
Copy link
Copy Markdown
Collaborator

With multiple folks from MS approving, I will be merging this.

@vettoreldaniele vettoreldaniele merged commit 0a1f7a1 into microsoft:main Nov 23, 2022
@ben-clayton ben-clayton deleted the fix-4786 branch November 23, 2022 17:03
hekota added a commit that referenced this pull request May 30, 2023
hekota added a commit that referenced this pull request May 31, 2023
…#5253)

This reverts commit 0a1f7a1.

Use an array of 32-bit integer values instead of 32-bit float values for the "dx.icb" immediate constant buffer data.
This prevents the FPU flipping the 22nd bit of integer constants, when their bit pattern represents a float32 signalling NaN on x86 architecture.

This change is causing rendering issues on PCs with AMD GPU and is blocking an upcoming Windows release.

The target branch of this revert is a release branch based on release-1.7.2212 at commit 83f8c6c and it will be used for the upcoming Windows release. The dxilconv change stays in main, at least for now.
hekota added a commit to hekota/DirectXShaderCompiler that referenced this pull request Jun 8, 2023
…ft#4790)" (microsoft#5253)

This reverts commit 0a1f7a1.

Use an array of 32-bit integer values instead of 32-bit float values for the "dx.icb" immediate constant buffer data.
This prevents the FPU flipping the 22nd bit of integer constants, when their bit pattern represents a float32 signalling NaN on x86 architecture.

This change is causing rendering issues on PCs with AMD GPU and is blocking an upcoming Windows release.

The target branch of this revert is a release branch based on release-1.7.2212 at commit 83f8c6c and it will be used for the upcoming Windows release. The dxilconv change stays in main, at least for now.

(cherry picked from commit 03df61d)
hekota added a commit to hekota/DirectXShaderCompiler that referenced this pull request Jun 8, 2023
…ft#4790)" (microsoft#5253)

This reverts commit 0a1f7a1.

Use an array of 32-bit integer values instead of 32-bit float values for the "dx.icb" immediate constant buffer data.
This prevents the FPU flipping the 22nd bit of integer constants, when their bit pattern represents a float32 signalling NaN on x86 architecture.

This change is causing rendering issues on PCs with AMD GPU and is blocking an upcoming Windows release.

The target branch of this revert is a release branch based on release-1.7.2212 at commit 83f8c6c and it will be used for the upcoming Windows release. The dxilconv change stays in main, at least for now.

(cherry picked from commit 03df61d)
hekota added a commit that referenced this pull request Jun 8, 2023
…#5253) (#5278)

This reverts commit 0a1f7a1.

Use an array of 32-bit integer values instead of 32-bit float values for the "dx.icb" immediate constant buffer data.
This prevents the FPU flipping the 22nd bit of integer constants, when their bit pattern represents a float32 signalling NaN on x86 architecture.

This change is causing rendering issues on PCs with AMD GPU and is blocking an upcoming Windows release.

The target branch of this revert is a release branch based on release-1.7.2212 at commit 83f8c6c and it will be used for the upcoming Windows release. The dxilconv change stays in main, at least for now.

(cherry picked from commit 03df61d)
hekota added a commit that referenced this pull request Jun 8, 2023
…#5253) (#5279)

This reverts commit 0a1f7a1.

Use an array of 32-bit integer values instead of 32-bit float values for the "dx.icb" immediate constant buffer data.
This prevents the FPU flipping the 22nd bit of integer constants, when their bit pattern represents a float32 signalling NaN on x86 architecture.

This change is causing rendering issues on PCs with AMD GPU and is blocking an upcoming Windows release.

The target branch of this revert is a release branch based on release-1.7.2212 at commit 83f8c6c and it will be used for the upcoming Windows release. The dxilconv change stays in main, at least for now.

(cherry picked from commit 03df61d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DxbcConverter can corrupt integer Immediate Constant Buffer values (x86)

5 participants