Skip to content

Fix invalid module bitcode when indexing a swizzled bool vector#6582

Merged
amaiorano merged 2 commits intomicrosoft:mainfrom
amaiorano:fix-indexing-swizzled-bool-vector
May 9, 2024
Merged

Fix invalid module bitcode when indexing a swizzled bool vector#6582
amaiorano merged 2 commits intomicrosoft:mainfrom
amaiorano:fix-indexing-swizzled-bool-vector

Conversation

@amaiorano
Copy link
Copy Markdown
Collaborator

When indexing a swizzled bool vector, some HLSL-specific code in EmitCXXMemberOrOperatorMemberCallExpr kicks in to handle the HLSLVecType. In this case, we’re dealing with an ExtVectorElt because of the swizzle, so this function creates a GEP, Load, and Store on the vector. However, boolean scalars are returned as type i11 while the store is storing to a bool, which is an i32, so we need to insert a cast before the store.

@amaiorano amaiorano requested a review from a team as a code owner May 3, 2024 22:09
@amaiorano
Copy link
Copy Markdown
Collaborator Author

See this example of the failure: https://godbolt.org/z/WsajscbT9

@amaiorano amaiorano requested review from adam-yang and llvm-beanz May 3, 2024 22:11
Comment thread tools/clang/test/CodeGenDXIL/operators/swizzle/indexSwizzledBoolVec.hlsl Outdated
Comment thread tools/clang/lib/CodeGen/CGExprCXX.cpp Outdated
When indexing a swizzled bool vector, some HLSL-specific code in
EmitCXXMemberOrOperatorMemberCallExpr kicks in to handle the
HLSLVecType. In this case, we’re dealing with an ExtVectorElt because of
the swizzle, so this function creates a GEP, Load, and Store on the
vector. However, we need to truncate the loaded value from i32 to i1,
otherwise we attempt to write an i32 to an i1, which trips the assert.
@amaiorano amaiorano force-pushed the fix-indexing-swizzled-bool-vector branch from 36a8db0 to 15e180b Compare May 6, 2024 18:33
Comment thread tools/clang/lib/CodeGen/CGExpr.cpp
Copy link
Copy Markdown
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

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

I think this looks good, but since I contributed a bit to it I want to get some additional eyes on it.

@farzonl
Copy link
Copy Markdown
Collaborator

farzonl commented May 7, 2024

LGTM

Copy link
Copy Markdown
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!

Comment thread tools/clang/lib/CodeGen/CGExpr.cpp Outdated
Comment thread tools/clang/lib/CodeGen/CGExpr.cpp
Comment thread tools/clang/lib/CodeGen/CGExprCXX.cpp
Comment thread tools/clang/lib/CodeGen/CGExpr.cpp Outdated
@amaiorano amaiorano merged commit 35de501 into microsoft:main May 9, 2024
qtprojectorg pushed a commit to qt/qtwebengine-chromium that referenced this pull request Sep 18, 2024
Manual cherry-pick of patch originally reviewed on
microsoft/DirectXShaderCompiler#6582:
Fix invalid module bitcode when indexing a swizzled bool vector (#6582)

When indexing a swizzled bool vector, some HLSL-specific code in
EmitCXXMemberOrOperatorMemberCallExpr kicks in to handle the
HLSLVecType. In this case, we’re dealing with an ExtVectorElt because of
the swizzle, so this function creates a GEP, Load, and Store on the
vector. However, boolean scalars are returned as type i11 while the
store is storing to a bool, which is an i32, so we need to insert a cast
before the store.

Change-Id: I4e2b8a46d4df29f7bbf24738509ff9095a31d8bf
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/591596
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
qtprojectorg pushed a commit to qt/qtwebengine-chromium that referenced this pull request Sep 18, 2024
Manual cherry-pick of patch originally reviewed on
microsoft/DirectXShaderCompiler#6582:
Fix invalid module bitcode when indexing a swizzled bool vector (#6582)

When indexing a swizzled bool vector, some HLSL-specific code in
EmitCXXMemberOrOperatorMemberCallExpr kicks in to handle the
HLSLVecType. In this case, we’re dealing with an ExtVectorElt because of
the swizzle, so this function creates a GEP, Load, and Store on the
vector. However, boolean scalars are returned as type i11 while the
store is storing to a bool, which is an i32, so we need to insert a cast
before the store.

Change-Id: I4e2b8a46d4df29f7bbf24738509ff9095a31d8bf
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/591589
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
SjMxr233 pushed a commit to ShaderHelper/DirectXShaderCompiler that referenced this pull request Jul 24, 2025
…osoft#6582)

When indexing a swizzled bool vector, some HLSL-specific code in
EmitCXXMemberOrOperatorMemberCallExpr kicks in to handle the
HLSLVecType. In this case, we’re dealing with an ExtVectorElt because of
the swizzle, so this function creates a GEP, Load, and Store on the
vector. However, boolean scalars are returned as type i11 while the
store is storing to a bool, which is an i32, so we need to insert a cast
before the store.
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.

5 participants