-
Notifications
You must be signed in to change notification settings - Fork 61
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
Better overload #1679
Better overload #1679
Conversation
FYI, this how I'd like "supports" to look
|
what's |
options{rbr::merge(op, defaults)}; |
bbc60af
to
d08f867
Compare
549bc97
to
cb9e54f
Compare
Applied most of the discussed changes. |
|
||
/// Default settings of eve::conditional is eve::ignore_none | ||
EVE_FORCEINLINE static constexpr auto defaults() noexcept { return options{condition_key = ignore_none}; } | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, do we want to just inherit relative_condition_option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about it but wasn't sure.
- Made rbr concepts usable in EVE - Fixed constant with else - Clean up here and there
0144458
to
14a7d0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will look more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will approve. I don't think we quite understand everything yet and there are some flaws I am seeing, but they need expirience
auto const f = Func<decltype(rmv_cond)>{rmv_cond}; | ||
|
||
// If the conditional call is supported, call it | ||
if constexpr( is_supported ) return detail::mask_op(cond, f, x0, xs...); |
There was a problem hiding this comment.
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 I understand how you plan to support conditionals.
You say is_supported
for a conditional call. What would that spec look like?
Alternatively - it should always be supported and this just keeps being in all overloads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup this is fishy, removign elementwise for now as discussed
constexpr bool any_simd = (simd_value<T> || ... || simd_value<Ts>); | ||
|
||
// Are we dealing with a no-condition call ? | ||
if constexpr( option_type_is<condition_key, O, ignore_none_> ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Ignore all support?
Do we have a uniform logic on how we handle ignores in element wise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a bit lacking indeed
After some more discussions, this implements a 3rd version of our overload system.