-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Closed
Labels
fixedSomething works now, yay!Something works now, yay!throughputMust compile fasterMust compile faster
Description
After @Neargye implemented constexpr <numeric> in #399, @miscco observed in #399 (comment) that there is a potential throughput concern here (and another occurrence below):
Lines 78 to 97 in 7ad0d63
| #ifdef __cpp_lib_is_constant_evaluated | |
| // TRANSITION, DevCom-878972 | |
| if (_STD is_constant_evaluated()) { | |
| for (; _UFirst != _ULast; ++_UFirst) { | |
| _Val = _Reduce_op(_STD move(_Val), *_UFirst); // Requirement missing from N4713 | |
| } | |
| return _Val; | |
| } else | |
| #endif // __cpp_lib_is_constant_evaluated | |
| { | |
| if constexpr (_Plus_on_arithmetic_ranges_reduction_v<_Unwrapped_t<const _InIt&>, _Ty, _BinOp>) { | |
| (void) _Reduce_op; // TRANSITION, VSO-486357 | |
| return _Reduce_plus_arithmetic_ranges(_UFirst, _ULast, _Val); | |
| } else { | |
| for (; _UFirst != _ULast; ++_UFirst) { | |
| _Val = _Reduce_op(_STD move(_Val), *_UFirst); // Requirement missing from N4713 | |
| } | |
| return _Val; | |
| } | |
| } |
On a related note, I observe that there is code repetition between the is_constant_evaluated and !_Plus_on_arithmetic_ranges_reduction_v paths.
I suspect that we can't get both optimal throughput and zero repetition (because of the non-if constexpr nature of is_constant_evaluated). So perhaps the best pattern is:
if constexpr (_Plus_on_arithmetic_ranges_reduction_v) {
if (!_STD is_constant_evaluated()) {
return _Reduce_plus_arithmetic_ranges(ARGS);
}
}
return CLASSIC_IMPLEMENTATION;
Metadata
Metadata
Assignees
Labels
fixedSomething works now, yay!Something works now, yay!throughputMust compile fasterMust compile faster