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

Masked compare and floating point classifications #2427

Merged

Conversation

mazimkhan
Copy link
Contributor

Introduces:

  • MaskedIsNaN(v): returns mask indicating whether v[i] is "not a number" (unordered) or false if m[i] is false.

  • MaskedIsInf(v): returns mask indicating whether v[i] is positive or negative infinity or false if m[i] is false.

  • MaskedIsFinite(v): returns mask indicating whether v[i] is neither NaN nor infinity, i.e. normal, subnormal or zero or false if m[i] is false. Equivalent to Not(Or(IsNaN(v), IsInf(v))).

  • MaskedCompEq(m, a, b), MaskedCompNe(m, a, b), MaskedCompLt(m, a, b), MaskedCompGt(m, a, b), MaskedCompLe(m, a, b), MaskedCompGe(m, a, b), returning the output for all versions of ==, !=, <, >, >=, <=, or false if m[i] is false.

All operations are written for both arm_sve-inl.h and generic_ops-inl.h and, equally, tests are included for all operations.

Copy link

google-cla bot commented Jan 6, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Member

@jan-wassenberg jan-wassenberg left a comment

Choose a reason for hiding this comment

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

Nice :) I think the CLA issue is still unresolved, any update there?

equivalent to, and potentially more efficient than, `And(m, Eq(a, b));` etc.

* `V`: `{f}` \
<code>M **MaskedIsNaN**(V v)</code>: returns mask indicating whether `v[i]`
Copy link
Member

Choose a reason for hiding this comment

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

Let's add the mask argument to the documentation :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies, have corrected.

#### Masked floating-point classification

All ops in this section return `false` for `mask=false` lanes. These are
equivalent to, and potentially more efficient than, `And(m, Eq(a, b));` etc.
Copy link
Member

Choose a reason for hiding this comment

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

And(m, IsNaN)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Very, true. Have corrected.

All ops in this section return `false` for `mask=false` lanes. These are
equivalent to, and potentially more efficient than, `And(m, Eq(a, b));` etc.

* <code>M **MaskedCompEq**(M m, V a, V b)</code>: returns `a[i] == b[i]` or
Copy link
Member

Choose a reason for hiding this comment

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

Should we just call it MaskedEq for consistency with the usual naming convention, which is only to prepend Masked to the existing Eq?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, have updated.

}

template <class V, class M, class D = DFromV<V>>
HWY_API MFromD<D> MaskedIsInf(const M m, const V v) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we ever plan to provide a faster implementation of MaskIfInf/IsFinite, or can those be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've removed these.

@mazimkhan
Copy link
Contributor Author

Nice :) I think the CLA issue is still unresolved, any update there?

Apologies, CLA issue has stuck around despite us signing it multiple times. Team was on holiday, now we are back and will resolve CLA and your comments soon.

@wbb-ccl wbb-ccl force-pushed the cc_up_masked_compare branch from 19ff43c to bfe4014 Compare January 29, 2025 17:25
mazimkhan and others added 3 commits January 29, 2025 17:29
Fix docs for masked floating-point classification
Rename MaskedComp* ops to Masked*
Remove redundant wrapper functions
@wbb-ccl wbb-ccl force-pushed the cc_up_masked_compare branch from bfe4014 to e8a840c Compare January 29, 2025 17:30
Copy link
Member

@jan-wassenberg jan-wassenberg left a comment

Choose a reason for hiding this comment

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

Nice, one small nit :)

@copybara-service copybara-service bot merged commit a74db0c into google:master Jan 31, 2025
38 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants