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 vector overloads for or and and #4529

Merged
merged 4 commits into from
Jul 5, 2024
Merged

Conversation

expipiplus1
Copy link
Collaborator

Closes #4441 and #4434

tests/bugs/gh-4434.slang Show resolved Hide resolved
Comment on lines +2263 to +2275
[__unsafeForceInlineEarly]
[OverloadRank(-10)]
vector<bool, N> and<let N : int>(bool b, vector<bool, N> v)
{
return and(vector<bool, N>(b), v);
}

[__unsafeForceInlineEarly]
[OverloadRank(-10)]
vector<bool, N> and<let N : int>(vector<bool, N> v, bool b)
{
return and(v, vector<bool, N>(b));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure if we should support this function that may lead to inefficient code.
A proper implementation for this should happen at the application level in a following manner,

vector<bool,N> result = (b? v : vector<bool,N>(false));

Copy link
Collaborator

Choose a reason for hiding this comment

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

@chaoticbob for an opinion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What makes you think it would be inefficient? Probably best left to the eventual backend to determine precisely what combination of conditional move and bitand is best.

I agree with not supporting it for different reasons (to avoid proliferation of overloads), but this ship has sailed because hlsl already supports this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm trying to catch up here, is the question here if a vectorized version of the and (and also or) intrinsic can lead to inefficient code?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm. Right.
I think "?:" is probably more efficient than vectorized "and" operation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If I'm reading this correctly, the question is about the efficiency of anding a vector and a scalar. For SPIR-V this compiles down an OpCompositeConstruct for the scalar to expand to a vector followed by OpLogicalAnd to perform the vectorized and operation. DXIL handles it with individual instructions per component.

As @expipiplus1 alluded to, it's difficult to gauge how efficient or inefficient this will be for the driver level shader compiler. Driver level shader compilers may have specialized instructions or patterns to handle these cases.

In addition to HLSL already supporting it, it's generally not good idea to be overly prescriptive about how application level code should be written, especially in cases where expression is awkwardly limited in the interest of potential efficiency.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok. I guess the downstream compiler will be able to optimize it out.

@ArielG-NV ArielG-NV added the pr: non-breaking PRs without breaking changes label Jul 3, 2024
@expipiplus1 expipiplus1 merged commit 65194cf into shader-slang:master Jul 5, 2024
17 checks passed
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.

Missing support for or intrinsic
4 participants