Skip to content

[SM6.9] Lower vector any/all calls to vector op#7753

Merged
V-FEXrt merged 9 commits intomicrosoft:mainfrom
V-FEXrt:7678-vectorized-any-all-lower
Sep 17, 2025
Merged

[SM6.9] Lower vector any/all calls to vector op#7753
V-FEXrt merged 9 commits intomicrosoft:mainfrom
V-FEXrt:7678-vectorized-any-all-lower

Conversation

@V-FEXrt
Copy link
Copy Markdown
Collaborator

@V-FEXrt V-FEXrt commented Sep 10, 2025

Fixes #7687

Implement vector reduction lowing for calls to any and all

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 10, 2025

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

Comment thread lib/HLSL/DxilScalarizeVectorIntrinsics.cpp Outdated
Copy link
Copy Markdown
Member

@damyanp damyanp left a comment

Choose a reason for hiding this comment

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

LGTM, but should aim for approval from someone more experienced than me.

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.

Few comments, plus the scalarization using just and needs to be updated.

Comment thread utils/hct/hctdb.py Outdated
Comment thread utils/hct/hctdb.py Outdated
Comment thread lib/HLSL/HLOperationLower.cpp Outdated
Comment on lines +1795 to +1799
bool NeedsConvertToInt = EltTy->isFloatingPointTy();
for (unsigned I = 0; I < Ty->getVectorNumElements(); I++) {
Value *Elt = Builder.CreateExtractElement(Arg, I);
if (NeedsConvertToInt)
Elt = GenerateCmpNEZero(Elt, Builder);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
bool NeedsConvertToInt = EltTy->isFloatingPointTy();
for (unsigned I = 0; I < Ty->getVectorNumElements(); I++) {
Value *Elt = Builder.CreateExtractElement(Arg, I);
if (NeedsConvertToInt)
Elt = GenerateCmpNEZero(Elt, Builder);
bool NeedsConvertToBool = EltTy->isFloatingPointTy();
for (unsigned I = 0; I < Ty->getVectorNumElements(); I++) {
Value *Elt = Builder.CreateExtractElement(Arg, I);
if (NeedsConvertToBool)
Elt = GenerateCmpNEZero(Elt, Builder);

Please rename, this is not really converting float value to integer ;)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

ack for the future! This actually goes away entirely with the latest commit!

Comment thread lib/HLSL/HLOperationLower.cpp
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 16, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@hekota
Copy link
Copy Markdown
Member

hekota commented Sep 16, 2025

The operations are declared as "Bitwise AND/OR reduction of the vector returning a scalar", but in reality, we only ever call it with a bool vector, right? I am not sure if that's a problem or not.

@V-FEXrt
Copy link
Copy Markdown
Collaborator Author

V-FEXrt commented Sep 16, 2025

The operations are declared as "Bitwise AND/OR reduction of the vector returning a scalar", but in reality, we only ever call it with a bool vector, right? I am not sure if that's a problem or not.

With the currently lowering, yes. It's only every called with an i1 vec. But there isn't a reason to add that restriction. For example, it might be nice to add an intrinsic to hlsl for "unary vector and" and "unary vector or" in the future but restricting the opcode would prevent that

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!

@V-FEXrt V-FEXrt merged commit 4e48d34 into microsoft:main Sep 17, 2025
12 checks passed
@github-project-automation github-project-automation Bot moved this from New to Done in HLSL Roadmap Sep 17, 2025
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.

[SM6.9] Vectorized lower any/all

3 participants